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

Deeper spotbugs checks #240

Merged
merged 11 commits into from
Sep 27, 2022
Merged

Conversation

dheerajodha
Copy link
Contributor

Increase the chance of detecting bugs

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did

@@ -42,6 +44,7 @@
*
* @author JO Sivtoft
*/
@SuppressFBWarnings(value="SIC_INNER_SHOULD_BE_STATIC_ANON")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should I add the "justification" field here inside the SuppressFBWarnings? If yes, then can anyone please tell me what exactly should I write here?

Copy link
Member

Choose a reason for hiding this comment

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

Does this refer to the customComparator? If so, the dumb fix would be to make it a static constant. More idiomatic in Java 8 would be to use https://docs.oracle.com/javase/8/docs/api/java/util/Comparator.html#comparing-java.util.function.Function-

Copy link
Contributor Author

@dheerajodha dheerajodha Mar 8, 2022

Choose a reason for hiding this comment

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

Hi @jglick thank you for looking into this PR!
However, I'd need a couple of days to get back to this as I was (and am a little) preoccupied with job interviews + GSoC's application phase. I appreciate your help

@MarkEWaite MarkEWaite enabled auto-merge (squash) September 27, 2022 01:39
@MarkEWaite MarkEWaite merged commit 8091841 into jenkinsci:master Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants