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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@

public class EndpointProcessingStep implements ProcessingStep {
private static final String GENERATED_ANNOTATION_JDK9 = "javax.annotation.processing.Generated";
private static final String GENERATED_ANNOTATION_LEGACY = "javax.annotation.Generated";
private static final String GENERATED_ANNOTATION_LEGACY = "jakarta.annotation.Generated";

private final ProcessingEnvironment processingEnv;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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?

private static final String GENERATED_ANNOTATION_LEGACY = "jakarta.annotation.Generated";

private TypeElement serializationStreamReader;
private TypeElement serializationStreamWriter;
Expand Down