PropertyEditors deprecated class.

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

PropertyEditors deprecated class.

Daniel Cunha
Hi Folks,

I've created the ticket https://issues.apache.org/jira/browse/TOMEE-2481

That because the class PropertyEditors is a deprected class, because of
that we are moving it to use the PropertyEditorRegistry as indicated on the
PropertyEditors comment: this is all static and leaks, use
PropertyEditorRegistry

PR is opened: https://github.com/apache/tomee/pull/427

Pleas review it.

--
Daniel "soro" Cunha
https://twitter.com/dvlc_
Reply | Threaded
Open this post in threaded view
|

Re: PropertyEditors deprecated class.

jgallimore
Interesting. Thanks for the PR - looking at it now.

Jon

On Thu, Feb 28, 2019 at 3:26 PM Daniel Cunha <[hidden email]> wrote:

> Hi Folks,
>
> I've created the ticket https://issues.apache.org/jira/browse/TOMEE-2481
>
> That because the class PropertyEditors is a deprected class, because of
> that we are moving it to use the PropertyEditorRegistry as indicated on the
> PropertyEditors comment: this is all static and leaks, use
> PropertyEditorRegistry
>
> PR is opened: https://github.com/apache/tomee/pull/427
>
> Pleas review it.
>
> --
> Daniel "soro" Cunha
> https://twitter.com/dvlc_
>
Reply | Threaded
Open this post in threaded view
|

Re: PropertyEditors deprecated class.

jgallimore
Added feedback on the PR, but including it here for visibility:

In LocalJMXCommand the PropertyEditorRegistry is held as a field. In other
places, this is doing PropertyEditorRegistry.registerDefaults() when
calling getValue(), which has to go an do a bunch of registration each time.

What do we think the lifecycle of a PropertyEditorRegistry should be? I did
a quick search for references, which throws up ManagedMBean,
ActiveMQ5Factory, JMSProducerImpl, and I would have thought it could be
tied to the lifecycle of the components using it in each case (i.e. - make
it a field).

Thoughts?

Jon

On Mon, Mar 4, 2019 at 10:34 AM Jonathan Gallimore <
[hidden email]> wrote:

> Interesting. Thanks for the PR - looking at it now.
>
> Jon
>
> On Thu, Feb 28, 2019 at 3:26 PM Daniel Cunha <[hidden email]> wrote:
>
>> Hi Folks,
>>
>> I've created the ticket https://issues.apache.org/jira/browse/TOMEE-2481
>>
>> That because the class PropertyEditors is a deprected class, because of
>> that we are moving it to use the PropertyEditorRegistry as indicated on
>> the
>> PropertyEditors comment: this is all static and leaks, use
>> PropertyEditorRegistry
>>
>> PR is opened: https://github.com/apache/tomee/pull/427
>>
>> Pleas review it.
>>
>> --
>> Daniel "soro" Cunha
>> https://twitter.com/dvlc_
>>
>
Reply | Threaded
Open this post in threaded view
|

Re: PropertyEditors deprecated class.

Daniel Cunha-2
Hi Jon,

thank you for your review, I'll take a look after we finish the changes for
java and have it merged on master


Em seg, 4 de mar de 2019 às 08:27, Jonathan Gallimore <
[hidden email]> escreveu:

> Added feedback on the PR, but including it here for visibility:
>
> In LocalJMXCommand the PropertyEditorRegistry is held as a field. In other
> places, this is doing PropertyEditorRegistry.registerDefaults() when
> calling getValue(), which has to go an do a bunch of registration each
> time.
>
> What do we think the lifecycle of a PropertyEditorRegistry should be? I did
> a quick search for references, which throws up ManagedMBean,
> ActiveMQ5Factory, JMSProducerImpl, and I would have thought it could be
> tied to the lifecycle of the components using it in each case (i.e. - make
> it a field).
>
> Thoughts?
>
> Jon
>
> On Mon, Mar 4, 2019 at 10:34 AM Jonathan Gallimore <
> [hidden email]> wrote:
>
> > Interesting. Thanks for the PR - looking at it now.
> >
> > Jon
> >
> > On Thu, Feb 28, 2019 at 3:26 PM Daniel Cunha <[hidden email]>
> wrote:
> >
> >> Hi Folks,
> >>
> >> I've created the ticket
> https://issues.apache.org/jira/browse/TOMEE-2481
> >>
> >> That because the class PropertyEditors is a deprected class, because of
> >> that we are moving it to use the PropertyEditorRegistry as indicated on
> >> the
> >> PropertyEditors comment: this is all static and leaks, use
> >> PropertyEditorRegistry
> >>
> >> PR is opened: https://github.com/apache/tomee/pull/427
> >>
> >> Pleas review it.
> >>
> >> --
> >> Daniel "soro" Cunha
> >> https://twitter.com/dvlc_
> >>
> >
>


--
Daniel "soro" Cunha
https://twitter.com/dvlc_