Hi Romain, yeah.. I can handle that.
Thank you! Em ter., 4 de fev. de 2020 às 02:48, Romain Manni-Bucau < [email protected]> escreveu: > 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_ >> > -- Daniel "soro" Cunha https://twitter.com/dvlc_
