-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
[JENKINS-60884] - Fix NPE in Run API when using getPreviousBuildsOverThreshold #4471
[JENKINS-60884] - Fix NPE in Run API when using getPreviousBuildsOverThreshold #4471
Conversation
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.
this is the method it is calling:
public @CheckForNull RunT getPreviousBuild() {
SpotBugs would have caught the bug if we were enforcing it 😢
Co-Authored-By: Zbynek Konecny <[email protected]>
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.
I plan to merge it tomorrow if no negative feedback
if (r != null) { | ||
return r.getBuildsOverThreshold(numberOfBuilds, threshold); | ||
} | ||
return new ArrayList<>(numberOfBuilds); |
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.
NIT-Non-blocking: Probably makes more sense for it to have no initial capacity
return new ArrayList<>(numberOfBuilds); | |
return new ArrayList<>(); |
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.
I agree but I chose to give it a capacity as this is what it would have returned in this case before being refactored.
My thought was to not change behavior no matter how insignificant. I can commit this suggestion if you still would recommend it after my comment.
Thanks for your contribution @AaronZurawski ! It should land in the next weekly |
Thanks everyone for the quick merge! |
…Threshold (jenkinsci#4471) * Added a null check to getPreviousBuildsOverThreshold. * Cleaned up accidental newline. * Update core/src/main/java/hudson/model/Run.java Co-Authored-By: Zbynek Konecny <[email protected]> Co-authored-by: Zbynek Konecny <[email protected]>
…Threshold (jenkinsci#4471) * Added a null check to getPreviousBuildsOverThreshold. * Cleaned up accidental newline. * Update core/src/main/java/hudson/model/Run.java Co-Authored-By: Zbynek Konecny <[email protected]> Co-authored-by: Zbynek Konecny <[email protected]>
See JENKINS-60884.
This is fix for a NullPointerException in Run.getPreviousBuildsOverThreshold that was introduced by a refactor to add a new Run.getBuildsOverThreshold method in revision PR #4259
Proposed changelog entries
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeMaintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate