Make Picocli version providers unremovable classes #38083
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
geoand
commented
Jan 8, 2024
•
geoand
commented
- Closes: picocli should be able to mark its annotation dependent classes as unremovable #38077
This comment has been minimized.
This comment has been minimized.
gsmet
reviewed
Jan 8, 2024
extensions/picocli/deployment/src/main/java/io/quarkus/picocli/deployment/PicocliProcessor.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
|
related to #38077 |
maxandersen
reviewed
Jan 8, 2024
extensions/picocli/deployment/src/main/java/io/quarkus/picocli/deployment/PicocliProcessor.java
Outdated
Show resolved
Hide resolved
maxandersen
reviewed
Jan 8, 2024
extensions/picocli/deployment/src/main/java/io/quarkus/picocli/deployment/PicocliProcessor.java
Outdated
Show resolved
Hide resolved
geoand
requested review from
maxandersen and
mkouba
and removed request for
mkouba
This comment has been minimized.
This comment has been minimized.
|
✔️ The latest workflow run for the pull request has completed successfully. It should be safe to merge provided you have a look at the other checks in the summary. You can consult the Develocity build scans. |
maxandersen
reviewed
Jan 9, 2024
| @@ -99,6 +101,17 @@ void picocliRunner(ApplicationIndexBuildItem applicationIndex, | |||
| additionalBean.produce(AdditionalBeanBuildItem.unremovableOf(DefaultPicocliCommandLineFactory.class)); | |||
| quarkusApplicationClass.produce(new QuarkusApplicationClassBuildItem(PicocliRunner.class)); | |||
| } | |||
|
|
|||
| // Make all classes that can be instantiated by IFactory unremovable | |||
| unremovableBean.produce(UnremovableBeanBuildItem.beanTypes(CommandLine.ITypeConverter.class, | |||
There was a problem hiding this comment.
Ok - so this is doing it for all instances even if not referenced by picocli config.
Just curious why you do it this way when we only have to do it for those pointed to by users annotation?
Ease of coding in quarkus or that you don't believe it will matter enough in extra memory used ?
There was a problem hiding this comment.
Ok - so this is doing it for all instances even if not referenced by picocli config.
All it does it make beans of this type unremovable - nothing more
There was a problem hiding this comment.
Just curious why you do it this way when we only have to do it for those pointed to by users annotation?
This way is much much cleaner and also makes chances of copy-paste errors in the code far lower.
geoand
added a commit
to geoand/quarkus
that referenced
this pull request
Jan 9, 2024
This makes the IDE warn extension authors when they have created a build item but have not used it. I encountered this in quarkusio#38083 when I wrote something like: ```java UnremovableBeanBuildItem.beanTypes(Foo.class, Bar.class) ``` in a hurry and didn't stop to consider that the build item was never being produced
gsmet
approved these changes
Jan 9, 2024
holly-cummins
pushed a commit
to holly-cummins/quarkus
that referenced
this pull request
Jul 31, 2024
This makes the IDE warn extension authors when they have created a build item but have not used it. I encountered this in quarkusio#38083 when I wrote something like: ```java UnremovableBeanBuildItem.beanTypes(Foo.class, Bar.class) ``` in a hurry and didn't stop to consider that the build item was never being produced
danielsoro
pushed a commit
to danielsoro/quarkus
that referenced
this pull request
Sep 20, 2024
This makes the IDE warn extension authors when they have created a build item but have not used it. I encountered this in quarkusio#38083 when I wrote something like: ```java UnremovableBeanBuildItem.beanTypes(Foo.class, Bar.class) ``` in a hurry and didn't stop to consider that the build item was never being produced
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.