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

Code clean-up: Add own private static final LOGGER in each class #26

Merged
merged 1 commit into from
May 13, 2018

Conversation

jmini
Copy link
Member

@jmini jmini commented May 13, 2018

Declaring LOGGER as private static final is a best practice in Java.

https://stackoverflow.com/questions/6653520/why-do-we-declare-loggers-static-final


This is part of code cleanup to avoid fields hiding others.

Having field declarations that hides another field or variable is in my opinion a bad practice in java. Name shadowing can cause subtle errors and should be avoided.

@jimschubert
Copy link
Member

This looks good, and is obviously something to keep an eye out for when reviewing pull requests.

I'm going to merge this now. As a related question, though, do you know if this is something we can catch with static analysis?

@jimschubert jimschubert merged commit ab9c4b5 into OpenAPITools:master May 13, 2018
@jmini jmini deleted the code_cleanup_logger branch May 13, 2018 19:09
@jmini
Copy link
Member Author

jmini commented May 13, 2018

As a related question, though, do you know if this is something we can catch with static analysis?

For "private static final Logger", maybe this can be achieved with Checkstyle (I did not check it).

For "avoiding fields hiding others" => an IDE with appropriate setting can spot stuff like that... But for PR we need something at build/review time.

Maybe a solution could be to introduce a tool like SonarQube (I hope they have an online hosted version with free plan for open-source, but I did not check it yet).

@jimschubert
Copy link
Member

I did a search and there's an slf4j plugin for Findbugs: https://github.com/KengoTODA/findbugs-slf4j that looks like it will do exactly this. It looks like only some of our sample code runs findbugs. I'll open an issue to generate discussion around static analysis options, since I'm really only familiar with those used in Scala projects.

@jmini jmini added this to the 3.0.0 milestone Jul 2, 2018
wing328 pushed a commit that referenced this pull request Feb 4, 2021
wing328 pushed a commit that referenced this pull request Jan 2, 2022
aserkes added a commit to aserkes/openapi-generator that referenced this pull request Jul 11, 2022
gradle support for SE server generator, fix junit tests, fix mainClass var in pom.xml

Signed-off-by: aserkes <[email protected]>
nilskuhn pushed a commit to nilskuhn/openapi-generator that referenced this pull request Apr 6, 2023
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.

2 participants