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

Clean up public non-final statics #14221

Merged
merged 2 commits into from
Feb 12, 2025
Merged

Conversation

msfroh
Copy link
Contributor

@msfroh msfroh commented Feb 11, 2025

Description

Following up on #14151 and #14152, I decided to grep for other public static non-final variables in the Lucene codebase.

This change makes statics final where it is both possible and seems appropriate. (That is, the variables really do seem to be constants.) I made a couple private instead, since they are modified, but privately.

I also updated some old-school code where IntelliJ flagged opportunities to use newer Java idioms.

Following up on
apache#14151 and
apache#14152, I decided to grep for
other non-final public static variables in the Lucene codebase.

This change makes statics final where it is both possible and seems
appropriate. (That is, the variables really do seem to be constants.)
Copy link
Contributor

@dweiss dweiss left a comment

Choose a reason for hiding this comment

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

Thanks! I think this can be merged as is, but I'll wait a bit if anybody else wants to chime in.

@msokolov
Copy link
Contributor

LGTM, thanks!

@risdenk
Copy link
Contributor

risdenk commented Feb 11, 2025

#14216 may help with this specifically NonFinalStaticField which was off since it was super noisy. https://errorprone.info/bugpattern/NonFinalStaticField

Also, renamed method to be more descriptive.
@msfroh
Copy link
Contributor Author

msfroh commented Feb 12, 2025

#14216 may help with this specifically NonFinalStaticField which was off since it was super noisy. https://errorprone.info/bugpattern/NonFinalStaticField

Once #14216 and this PR are merged, I'm happy to make the required changes to enable the NonFinalStaticField check. It should be a lot less noisy.

@dweiss dweiss merged commit 33f314f into apache:main Feb 12, 2025
6 checks passed
@dweiss
Copy link
Contributor

dweiss commented Feb 12, 2025

Done. Thank you!

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

Successfully merging this pull request may close these issues.

4 participants