-
Notifications
You must be signed in to change notification settings - Fork 43
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
fix eclipse deprecation warnings #1917
Conversation
Remove @SuppressWarnings("deprecation") on deprecated methods. The annotation is for suppressing calls of deprecated methods, not implementation of deprecated methods. partial fix for #1635
Generate changelog in
|
@@ -305,7 +305,6 @@ private int getValue() { | |||
} | |||
|
|||
@Override | |||
@SuppressWarnings("deprecation") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct? I would have though a non-deprecated method (accept) calling a deprecated method (Visitor::visitBar) would be problematic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hypothesis: if the deprecated method is defined in the same file, then the usage does not produce a warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iirc we introduced the suppressions because we use javac -Xlint:deprecation
in some places where it may apply to generated code. I'd like to ensure we're not regressing that functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, then would you be willing to approve something similar to palantir/gradle-conjure#951? I'm thinking of suppressing all warnings from the generated project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that’s reasonable given the warnings may not necessarily align with deprecation checks elsewhere.
Doing this instead palantir/gradle-conjure#1136 |
Remove
@SuppressWarnings("deprecation")
on deprecated methods. The annotation is for suppressing calls of deprecated methods, not implementation of deprecated methods.partial fix for #1635
Splitting out from #1916
Before this PR
Generated code produces warnings in eclipse
After this PR
==COMMIT_MSG==
Generated code does not produce deprecation warnings in eclipse by default
==COMMIT_MSG==
Possible downsides?
Not tested in intellij. Only tested on one internal repo.