-
-
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-62723] Fix behaviour of Util.isOverridden() #4833
Conversation
It will now deal with final/abstract methods appropriately. It will also clearly report bad arguments (base not base of derived, or base does not declare the method). Added a unit test for interface cases. Small gap to be looked into: overrides provided via default implementation of a (derived) interface.
This handles the remaining case (override provided by default implementation of an interface).
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.
Looks fine from a two-minute glance; did not study implementation deeply.
Does it fix the known bug?
This PR is ready to merge, and will be merged after 24 hours if there's no objection |
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.
+1 from me, the change looks good. Thanks @Zastai !
I will merge it once the CI run passes |
(cherry picked from commit f7ae11e)
See JENKINS-62723.
Based on #4832 (which provides a unit test reproducing the issue)
Proposed changelog entries
IllegalArgumentException: Method not found
error caused by misbehaviour inUtil.isOverridden()
(regression in 2.241)Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeDesired reviewers
@jglick
Maintainer 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
to be considered (see query).