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_
