-
Notifications
You must be signed in to change notification settings - Fork 123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add more Exception messages for Config.getValue() #504
Conversation
9f6f45c
to
f0d8b45
Compare
@Joseph-Cass Is this ready? |
This comment may change depending on the outcome of this discussion, but other than that, I'm happy if you are :) |
implementation/src/main/java/io/smallrye/config/SmallRyeConfig.java
Outdated
Show resolved
Hide resolved
implementation/src/main/java/io/smallrye/config/ConfigMessages.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Joseph Cass <[email protected]>
Signed-off-by: Joseph Cass <[email protected]>
39c034b
to
d091ce3
Compare
Does this look okay to you @radcortez ? |
Yes! Thank you! |
Cannot backport to 1.x due to merge conflicts. Please backport manually:
To fix the conflict, first check which file is impacted using: Once done and pushed, open the pull request.
|
@radcortez I'm not sure this PR is suitable for 1.x. It updates error messages related to "empty" and null behaviour which changed in the spec between MP Config 1.x -> 2.0? |
Ahah, I was actually using this as a test for a script I wrote to do backports. There are some interesting things that I would like to backport, but I haven't really tried it yet. |
* Add more Exception messages for Config.getValue() Signed-off-by: Joseph Cass <[email protected]>
* Add more Exception messages for Config.getValue() Signed-off-by: Joseph Cass <[email protected]>
Partly addresses #485
In particular, resolves:
Under the current implementation all of the tests from
ConfigValueConversionRulesExceptionsTest.java
would have thrownExcept from
badConversion()
which would have thrown: