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_
