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

[build] Add unused public or protected field to spotbugs exclude #6957

Closed
wants to merge 1 commit into from

Conversation

spacey-sooty
Copy link
Contributor

No description provided.

@spacey-sooty spacey-sooty requested a review from a team as a code owner August 12, 2024 07:51
@sciencewhiz
Copy link
Contributor

What problem does this solve? It generally doesn't seem like a good thing to exclude.

@rzblue
Copy link
Member

rzblue commented Aug 12, 2024

If a field is public it's likely intended to be used outside of the current class, which spotbugs won't see.

@calcmogul
Copy link
Member

iirc, one of our tools besides spotbugs reports unused private fields.

@ThadHouse
Copy link
Member

I think this specific case is getting confused since it’s used by the annotation processor, which spotless doesn’t see.

@spacey-sooty
Copy link
Contributor Author

iirc, one of our tools besides spotbugs reports unused private fields.

Unused private fields makes sense to report, public and protected fields can be used outside of the class such as in #6893 which is failing because of this warning

@sciencewhiz
Copy link
Contributor

sciencewhiz commented Aug 13, 2024

Unused private fields makes sense to report, public and protected fields can be used outside of the class such as in #6893 which is failing because of this warning

It still seems pretty rare that you'd have a public variable that is never used within that class. Why not just suppress those two occurrences in that PR? Beachbot330@b826dee

@spacey-sooty
Copy link
Contributor Author

It still seems like a silly warning to me; I'll supress those for now though

@spacey-sooty
Copy link
Contributor Author

spacey-sooty commented Aug 13, 2024

Supressing warnings actually gives us a choice to make. How do we want to add the annotation?
Adding it as a dependency is a pain (we need to hold the version to spotbugs to the manually)
If we don't add it as a dependency we have to support the annotation ourselves.

@spacey-sooty spacey-sooty deleted the unusedfield branch August 14, 2024 00:59
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.

5 participants