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.

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

User jakarta @Generated annotations #58

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

JamesXNelson
Copy link
Contributor

This may not even be necessary, just what I did locally while working on jetty upgrade

@@ -45,7 +45,7 @@ public class Processor extends AbstractProcessor {
private static final String knownTypesFilename = "knownTypes.txt";

private static final String GENERATED_ANNOTATION_JDK9 = "javax.annotation.processing.Generated";
private static final String GENERATED_ANNOTATION_LEGACY = "javax.annotation.Generated";
Copy link
Member

Choose a reason for hiding this comment

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

I'm not necessarily opposed to this, but given how this is consumed (addGeneratedMetadata, about line 746), shouldnt it be a new annotation, so we can ask "do you have A, B, or C" and use the first that is available?

That is, introduce a new constant, and add it to the stream to be checked. I've confirmed that the javax.annotation.processing.Generated support in GWT works now, and can be restored.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I was thinking the same thing. Even something as simple as System.getProperty("generated.annotation", "javax.annotation.Generated") would be nice. Certainly possible to somehow skim the "classpath" / codegen environment to find types, but if there was more than one match, you'd probably want the system property anyway to decide, so unlikely to be worth the effort.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Basically, this was "I did this for internal changes using this code", knowing it wasn't suitable to merge (my bad for not explaining that). I defer to you on what solution you think is most maintainable. Keeping the default gwt compatible is obviously best. I did not have any issues building dh js api locally w/ this change, so jakarta is probably fine too. It has SOURCE retention, so I think that means gwt only needs the annotation class and not the source, no?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants