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

Use applied directives rather than directives by themselves #72

Merged
merged 1 commit into from
Sep 15, 2022

Conversation

pelletier197
Copy link
Contributor

Update to use the new AppliedDirectives instead of the old Directive.

This seems to break when upgrading to graphql-java 19, and it does still work for graphql-java 18.

@pelletier197
Copy link
Contributor Author

@enriquedacostacambio any chance we can get this merged if it's okay? This is stopping us from updating to the latest GraphQL tools.

Thanks in advance!

import jakarta.validation.Payload;
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import java.util.*;
Copy link
Member

Choose a reason for hiding this comment

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

This annoys me - please turn on expand all imports

Copy link
Member

@bbakerman bbakerman left a comment

Choose a reason for hiding this comment

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

I am happy to accept this PR howev er I would really like it to not use .* imports on the changed files.

Could you please go back and re-format this please

@bbakerman
Copy link
Member

Ok progress over perfection - I will let this in and we can fix up the .* imports later - could I ask that another PR be raised for that please @pelletier197

@bbakerman bbakerman merged commit 598e375 into graphql-java:master Sep 15, 2022
@pelletier197
Copy link
Contributor Author

Ok progress over perfection - I will let this in and we can fix up the .* imports later - could I ask that another PR be raised for that please @pelletier197

Yeah no problem, I'll do that today. Sorry about that, it's the default setting of Intellij and I didn't see it.

@dondonz dondonz added this to the 19.0 milestone Sep 26, 2022
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.

4 participants