Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Allow for usage of @ConfigProperty on fields without @Inject #1554

Merged
merged 1 commit into from
Mar 18, 2019
Merged

Allow for usage of @ConfigProperty on fields without @Inject #1554

merged 1 commit into from
Mar 18, 2019

Conversation

manovotn
Copy link
Contributor

@manovotn manovotn commented Mar 18, 2019 •
edited by cescoffier
Loading

No description provided.

michalszynkiewicz
michalszynkiewicz reviewed Mar 18, 2019
View reviewed changes
Copy link
Member

@michalszynkiewicz michalszynkiewicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me but I'd rather wait for someone experienced with Quarkus to approve

mkouba
mkouba requested changes Mar 18, 2019
View reviewed changes
Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why a new Processor class? I would move the method to the ConfigBuildStep. Otherwise looks good?

@manovotn manovotn requested a review from mkouba March 18, 2019 12:19
@manovotn
Copy link
Contributor Author

@mkouba good point, I didn't realize I can stuff multiple build steps in there.
Changed, squashed and pushed.

@mkouba
Copy link
Contributor

mkouba commented Mar 18, 2019

Looks good now. Let's wait for CI checks...

@mkouba mkouba merged commit b2d51f2 into quarkusio:master Mar 18, 2019
geoand added a commit to geoand/quarkus-quickstarts that referenced this pull request Mar 18, 2019
@geoand
Remove now unnecessary @Inject on @ConfigProperty
0cbde61 
quarkusio/quarkus#1554 made it redundant to have
@Inject on a @ConfigProperty
@geoand geoand mentioned this pull request Mar 18, 2019
Merged
geoand added a commit to geoand/quarkus that referenced this pull request Mar 18, 2019
@geoand
Remove now unnecessary @Inject on @ConfigProperty
18d9665 
quarkusio#1554 made it redundant to have @Inject on a @ConfigProperty

Follows up on: quarkusio#1315
@geoand geoand mentioned this pull request Mar 18, 2019
Merged
geoand added a commit to geoand/quarkus that referenced this pull request Mar 18, 2019
@geoand
Remove now unnecessary @Inject on @ConfigProperty
905dd5f 
quarkusio#1554 made it redundant to have @Inject on a @ConfigProperty

Follows up on: quarkusio#1315
@gsmet gsmet added this to the 0.12.0 milestone Mar 19, 2019
@cescoffier cescoffier changed the title Allow for usage of @ConfigProperty on fields without the need for @In… Allow for usage of @ConfigProperty on fields without @Inject Mar 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@michalszynkiewicz michalszynkiewicz michalszynkiewicz left review comments

@mkouba mkouba mkouba approved these changes

Assignees
No one assigned
Labels
release/noteworthy-feature
Projects
None yet
Milestone
0.12.0
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@manovotn @mkouba @michalszynkiewicz @cescoffier @gsmet