GERONIMO-6759 - Support for MicroProfile Config 1.4

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

GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha
Hi Folks,

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
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 <[hidden email]> a écrit :
Hi Folks,

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
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha
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 <[hidden email]> 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 <[hidden email]> a écrit :
Hi Folks,

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


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Hi Folks,

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


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha
Changes sent!

Thank you for your review Romain.

Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Hi Folks,

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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 <[hidden email]> a écrit :
Changes sent!

Thank you for your review Romain.

Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Hi Folks,

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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha
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 <[hidden email]> 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 <[hidden email]> a écrit :
Changes sent!

Thank you for your review Romain.

Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Hi Folks,

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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
Merged, thks a lot Daniel

Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Changes sent!

Thank you for your review Romain.

Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Hi Folks,

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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 BlogGithub | LinkedIn | Book


Le dim. 26 janv. 2020 à 08:28, Romain Manni-Bucau <[hidden email]> a écrit :
Merged, thks a lot Daniel

Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Changes sent!

Thank you for your review Romain.

Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
Hi Folks,

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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Mark Struberg
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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_

Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha-2
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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha-2
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha-2
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha-2

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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 |  Blog | Old BlogGithub | LinkedIn | Book


Le sam. 1 févr. 2020 à 10:34, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha-2
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 <[hidden email]> 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 |  Blog | Old BlogGithub | LinkedIn | Book


Le sam. 1 févr. 2020 à 10:34, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Romain Manni-Bucau
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 <[hidden email]> 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 <[hidden email]> 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 |  Blog | Old BlogGithub | LinkedIn | Book


Le sam. 1 févr. 2020 à 10:34, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
Reply | Threaded
Open this post in threaded view
|

Re: GERONIMO-6759 - Support for MicroProfile Config 1.4

Daniel Cunha-2
Hmm..

Good catch! So, +1 to proceed with your strategy.

Em sáb., 1 de fev. de 2020 às 07:39, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 |  Blog | Old BlogGithub | LinkedIn | Book


Le sam. 1 févr. 2020 à 10:34, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> escreveu:
lgtm,
Thanks Daniel and also Romain!

LieGrue,
strub


> Am 26.01.2020 um 16:22 schrieb Romain Manni-Bucau <[hidden email]>:
>
> 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 <[hidden email]> a écrit :
> Merged, thks a lot Daniel
>
> Le dim. 26 janv. 2020 à 01:25, Daniel Cunha <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> a écrit :
> Changes sent!
>
> Thank you for your review Romain.
>
> Em sáb., 25 de jan. de 2020 às 15:05, Romain Manni-Bucau <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha


--
Daniel "soro" Cunha
12