-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Show nicer error when a @ConfigMapping class exists in the deployment module and set to the runtime phase #43513
Conversation
Thanks for your pull request! Your pull request does not follow our editorial rules. Could you have a look?
This message is automatically generated by a bot. |
Hi @radcortez, could you see if I am in the right path? Any suggestions are welcome! |
9c35813
to
a1a4faf
Compare
core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java
Outdated
Show resolved
Hide resolved
core/deployment/src/main/java/io/quarkus/deployment/steps/ConfigGenerationBuildStep.java
Outdated
Show resolved
Hide resolved
Thank you for the PR; I've left a few comments. |
a1a4faf
to
58ad200
Compare
58ad200
to
bf90dd3
Compare
...ployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java
Outdated
Show resolved
Hide resolved
1b003f3
to
363dafd
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CI failure is related.
005cb51
to
50902a7
Compare
...ployment/src/main/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReader.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add a test please? Thank you.
Sorry for delay @radcortez, I will use this wekeend to add it! |
272657c
to
c396af0
Compare
...ment/src/test/java/io/quarkus/deployment/configuration/BuildTimeConfigurationReaderTest.java
Outdated
Show resolved
Hide resolved
...ests/test-extension/extension-that-defines-runtime-config-at-build/disable-unbind-executions
Outdated
Show resolved
Hide resolved
integration-tests/test-extension/extension-that-defines-runtime-config-at-build/runtime/pom.xml
Outdated
Show resolved
Hide resolved
bb77951
to
caf5c3e
Compare
This comment has been minimized.
This comment has been minimized.
099c485
to
18e849c
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
18e849c
to
71047bd
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Looks like CI is passing! Could you squash the commits? I'm traveling for 2 days, will have a look after that if noone beats me to it. |
Don't worry about it. I'll have a look. |
Done! |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
I gave it a try, and it worked as expected. I think this last implementation is way better than what we had before. Thank you for the work and patience @mcruzdev |
I am the one who is grateful :) |
...ava/io/quarkus/annotation/processor/documentation/config/scanner/AbstractConfigListener.java
Outdated
Show resolved
Hide resolved
...java/io/quarkus/annotation/processor/documentation/config/scanner/ConfigMappingListener.java
Outdated
Show resolved
Hide resolved
a953814
to
c43e0dc
Compare
Status for workflow
|
Status for workflow
|
@ConfigMapping
class exists in thedeployment
module and set to theruntime
phase #43394