-
Notifications
You must be signed in to change notification settings - Fork 25
Add recipe to change logger fields to private #221
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
Add recipe to change logger fields to private #221
Conversation
src/test/java/org/openrewrite/java/logging/ChangeLoggersToPrivateTest.java
Show resolved
Hide resolved
src/test/java/org/openrewrite/java/logging/ChangeLoggersToPrivateTest.java
Show resolved
Hide resolved
src/main/java/org/openrewrite/java/logging/ChangeLoggersToPrivate.java
Outdated
Show resolved
Hide resolved
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.
Thanks a lot for the help here @jhl221123 ! I've added some light polishing commits. Preconditions should help keep this recipe performant, by only traversiong through compilation units that use a logger at all. The ListUtils is a slightly more expressive way of manipulating elements. And the prefix wrangling that was there previously is no longer necessary. I've also added your recipe to be included with our SLF4J best practices, such that folks do no have to specifically seek out this recipe to get the benefits, but can run the broader best practices.
We acknowledge that if folks were to use the logger field from another class that might now break; we can optionally follow up with a change to make this a scanning recipe that detects such usage, and in those cases does not make any changes. I'd consider that rare though, and tolerable for a best practices recipe.
…ate.java Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@timtebeek, Thanks for the polishing commits and for merging this! I've learned a lot more about OpenRewrite thanks to your guidance during this process. I'll be sure to keep these points in mind for even better contributions next time. Thank you! |
Thanks a lot! Looking forward to what you'll do next. :D |
What's changed?
This PR introduces
ChangeLoggersToPrivate
, a new recipe enforcing private visibility for logger fields.What's your motivation?
Addresses issue #219, promoting better encapsulation of logging mechanics.
Anything in particular you'd like reviewers to focus on?
Correctness of modifier changes across different logger types and existing modifiers.
Checklist