Hi Daniel, Thanks for the update. This is an iso-change, thought we were moving null handling to ConfigImpl rather than ConfigInjectionBean to enable it in Config.get[Optional]Value too (just requires an interval getRawValue for injection but nothing crazy), this is why i spoke about the constant in ConfigImpl and didnt expect Defaults class. Do you want to handle the ConfigImpl change? If not i can follow up.
Le mar. 4 févr. 2020 à 02:58, Daniel Cunha <[email protected]> a écrit : > I had updated the PR: > https://github.com/apache/geronimo-config/pull/7/files > Hope it sounds good now. > > Em sáb., 1 de fev. de 2020 às 08:25, Daniel Cunha <[email protected]> > escreveu: > >> Hmm.. >> >> Good catch! So, +1 to proceed with your strategy. >> >> Em sáb., 1 de fev. de 2020 às 07:39, Romain Manni-Bucau < >> [email protected]> escreveu: >> >>> Not really, half of the cases will be in databases or properties file so >>> the constant does not help. >>> Other half is in config property but if you use a spec you dont want to >>> depend directly on the impl so better to not expose it yet IMHO (it will be >>> fixed in the spec in 2.0 anyway). >>> That said ill not fight against a constant in ConfigImpl. >>> >>> Le sam. 1 févr. 2020 à 11:04, Daniel Cunha <[email protected]> a >>> écrit : >>> >>>> Ok, I get it. >>>> >>>> But the user should define that value in the ConfigProperty. I mean >>>> that with the constant it keep more fancy/easy/safe to the user. The >>>> constant is just a simplify way to not need to memorize the property like: >>>> org.apache.geronimo.config.value.NULL. >>>> It's better to use Constants.NULL_VALUE instead of set >>>> org.apache.geronimo.config.value.NULL in the ConfigProperty. IMHO. >>>> >>>> Em sáb., 1 de fev. de 2020 às 06:38, Romain Manni-Bucau < >>>> [email protected]> escreveu: >>>> >>>>> The only location we need that constant is there: >>>>> https://github.com/apache/geronimo-config/blob/trunk/impl/src/main/java/org/apache/geronimo/config/ConfigImpl.java#L135 >>>>> >>>>> Something like return >>>>> "org.apache.geronimo.config.value.NULL".equals(value) ? null : value; >>>>> >>>>> We also don't want the users to import >>>>> org.apache.geronimo.config.value.Constants in their code so not sure a >>>>> constant or enum is needed, was just that. >>>>> >>>>> Romain Manni-Bucau >>>>> @rmannibucau <https://twitter.com/rmannibucau> | Blog >>>>> <https://rmannibucau.metawerx.net/> | Old Blog >>>>> <http://rmannibucau.wordpress.com> | Github >>>>> <https://github.com/rmannibucau> | LinkedIn >>>>> <https://www.linkedin.com/in/rmannibucau> | Book >>>>> <https://www.packtpub.com/application-development/java-ee-8-high-performance> >>>>> >>>>> >>>>> Le sam. 1 févr. 2020 à 10:34, Daniel Cunha <[email protected]> a >>>>> écrit : >>>>> >>>>>> >>>>>> Not sure if I get your point and applicability of it. >>>>>> >>>>>> Em sáb., 1 de fev. de 2020 às 04:49, Romain Manni-Bucau < >>>>>> [email protected]> escreveu: >>>>>> >>>>>>> Normally it should be used in a single place in ConfigImpl - dont >>>>>>> think we want users to see it, g-config should stay in scope runtime in >>>>>>> projects - so hardcoding it is ok and simpler to read but not a big >>>>>>> deal if >>>>>>> it is a constant. >>>>>>> >>>>>>> Le sam. 1 févr. 2020 à 04:27, Daniel Cunha <[email protected]> a >>>>>>> écrit : >>>>>>> >>>>>>>> Hey, >>>>>>>> >>>>>>>> I saw your comment on my PR. >>>>>>>> So, I like the idea to keep it as you mentioned, maybe we can move >>>>>>>> it for an Interface with some constants or better an Enum. >>>>>>>> So we can keep the NULL_VALUES still on the game in Geronimo, since >>>>>>>> it was removed in 1.4-RC3. :) >>>>>>>> >>>>>>>> So, if the spec choose to continue with a strategy like NULL_VALUES >>>>>>>> we'll continue support spec and our implementation as well. >>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> Em sex., 31 de jan. de 2020 às 23:18, Daniel Cunha < >>>>>>>> [email protected]> escreveu: >>>>>>>> >>>>>>>>> Sounds good. I'll update the PR for it. :) >>>>>>>>> >>>>>>>>> -- >>>>>>>>> Daniel "soro" Cunha >>>>>>>>> https://twitter.com/dvlc_ >>>>>>>>> >>>>>>>>> On Fri, Jan 31, 2020, 18:35 Romain Manni-Bucau < >>>>>>>>> [email protected]> wrote: >>>>>>>>> >>>>>>>>>> Guess we can just use a custom geronimo constant and keep the >>>>>>>>>> feature. It is needed in a lot of apps anyway and we dont need to >>>>>>>>>> break it >>>>>>>>>> in mpconfig 2 when the spec will have another solution. >>>>>>>>>> >>>>>>>>>> We should also wire it in Config to be able to reset a value >>>>>>>>>> (using source ordinals). >>>>>>>>>> >>>>>>>>>> Wdyt? >>>>>>>>>> >>>>>>>>>> Le ven. 31 janv. 2020 à 22:26, Daniel Cunha <[email protected]> >>>>>>>>>> a écrit : >>>>>>>>>> >>>>>>>>>>> Hi Folks, >>>>>>>>>>> >>>>>>>>>>> Changes for MicroProfile Config 1.4-RC3. PR: >>>>>>>>>>> https://github.com/apache/geronimo-config/pull/7 >>>>>>>>>>> The NULL_VALUE was reverted. TCK and our tests is passing as >>>>>>>>>>> expected. :) >>>>>>>>>>> >>>>>>>>>>> Best regard >>>>>>>>>>> >>>>>>>>>>> Em dom., 26 de jan. de 2020 às 17:20, Mark Struberg < >>>>>>>>>>> [email protected]> escreveu: >>>>>>>>>>> >>>>>>>>>>>> lgtm, >>>>>>>>>>>> Thanks Daniel and also Romain! >>>>>>>>>>>> >>>>>>>>>>>> LieGrue, >>>>>>>>>>>> strub >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>>> > Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau < >>>>>>>>>>>> [email protected]>: >>>>>>>>>>>> > >>>>>>>>>>>> > FYI I just fixed master code - test was using the proxy >>>>>>>>>>>> fields instead of injected values. Feel free to review and enhance >>>>>>>>>>>> if >>>>>>>>>>>> needed. >>>>>>>>>>>> > >>>>>>>>>>>> > Romain Manni-Bucau >>>>>>>>>>>> > @rmannibucau | Blog | Old Blog | Github | LinkedIn | Book >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > Le dim. 26 janv. 2020 à 08:28, Romain Manni-Bucau < >>>>>>>>>>>> [email protected]> a écrit : >>>>>>>>>>>> > Merged, thks a lot Daniel >>>>>>>>>>>> > >>>>>>>>>>>> > Le dim. 26 janv. 2020 à 01:25, Daniel Cunha < >>>>>>>>>>>> [email protected]> a écrit : >>>>>>>>>>>> > I believe now it's in a good shape. >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > Thank you, Romain. >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > -- >>>>>>>>>>>> > Daniel "soro" Cunha >>>>>>>>>>>> > https://twitter.com/dvlc_ >>>>>>>>>>>> > >>>>>>>>>>>> > On Sat, Jan 25, 2020, 17:55 Romain Manni-Bucau < >>>>>>>>>>>> [email protected]> wrote: >>>>>>>>>>>> > You dont need to parse constants : >>>>>>>>>>>> > >>>>>>>>>>>> > Long.parseLong("0") -> 0L ;) >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > Otherwise looks perfect for me >>>>>>>>>>>> > If nobody shouts, i will merge it tmr or on monday >>>>>>>>>>>> > >>>>>>>>>>>> > Le sam. 25 janv. 2020 à 20:10, Daniel Cunha < >>>>>>>>>>>> [email protected]> a écrit : >>>>>>>>>>>> > Changes sent! >>>>>>>>>>>> > >>>>>>>>>>>> > Thank you for your review Romain. >>>>>>>>>>>> > >>>>>>>>>>>> > Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau < >>>>>>>>>>>> [email protected]> escreveu: >>>>>>>>>>>> > Proxy supports primitives so default is not always null >>>>>>>>>>>> compared to injections, no? Once this point materialized by a test >>>>>>>>>>>> - and >>>>>>>>>>>> maybe imports reorganized to minimize the diff? - i guess we are >>>>>>>>>>>> good to >>>>>>>>>>>> merge. >>>>>>>>>>>> > >>>>>>>>>>>> > Le sam. 25 janv. 2020 à 18:37, Daniel Cunha < >>>>>>>>>>>> [email protected]> a écrit : >>>>>>>>>>>> > I updated the PR. Hope it is in a good shape now! >>>>>>>>>>>> > >>>>>>>>>>>> > Thank you. >>>>>>>>>>>> > >>>>>>>>>>>> > Em sáb., 25 de jan. de 2020 às 13:08, Romain Manni-Bucau < >>>>>>>>>>>> [email protected]> escreveu: >>>>>>>>>>>> > Except a small import issue (*) i guess it just needs the >>>>>>>>>>>> proxy handling (in our invocation handler)of default value and >>>>>>>>>>>> some test(s) >>>>>>>>>>>> then it looks pretty good to me. >>>>>>>>>>>> > >>>>>>>>>>>> > Le sam. 25 janv. 2020 à 17:01, Daniel Cunha < >>>>>>>>>>>> [email protected]> a écrit : >>>>>>>>>>>> > Hi Folks, >>>>>>>>>>>> > >>>>>>>>>>>> > https://github.com/apache/geronimo-config/pull/6 >>>>>>>>>>>> > >>>>>>>>>>>> > That is the PR with changes to cover MicroProfile 1.4-RC on >>>>>>>>>>>> Geronimo Config. >>>>>>>>>>>> > I really appreciate if someone could put the eyes on it. >>>>>>>>>>>> > >>>>>>>>>>>> > Thank you. >>>>>>>>>>>> > >>>>>>>>>>>> > -- >>>>>>>>>>>> > Daniel "soro" Cunha >>>>>>>>>>>> > https://twitter.com/dvlc_ >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > -- >>>>>>>>>>>> > Daniel "soro" Cunha >>>>>>>>>>>> > https://twitter.com/dvlc_ >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > -- >>>>>>>>>>>> > Daniel "soro" Cunha >>>>>>>>>>>> > https://twitter.com/dvlc_ >>>>>>>>>>>> >>>>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> -- >>>>>>>>>>> Daniel "soro" Cunha >>>>>>>>>>> https://twitter.com/dvlc_ >>>>>>>>>>> >>>>>>>>>> >>>>>>>> >>>>>>>> -- >>>>>>>> Daniel "soro" Cunha >>>>>>>> https://twitter.com/dvlc_ >>>>>>>> >>>>>>> >>>>>> >>>>>> -- >>>>>> Daniel "soro" Cunha >>>>>> https://twitter.com/dvlc_ >>>>>> >>>>> >>>> >>>> -- >>>> Daniel "soro" Cunha >>>> https://twitter.com/dvlc_ >>>> >>> >> >> -- >> Daniel "soro" Cunha >> https://twitter.com/dvlc_ >> > > > -- > Daniel "soro" Cunha > https://twitter.com/dvlc_ >
