-
Notifications
You must be signed in to change notification settings - Fork 354
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
Fix binary compatibility for PullRequestSCMRevision.getPull #817
Conversation
@andrey-fomin For some reason I can't request you as a reviewer, so here is a normal ping. 🤷 |
For me this fix looks ok. |
Method `getPull` is used in some private plugins. jenkinsci#796 changed signature of this method. Now signature is returned back. See also jenkinsci#817.
Method `getPull` is used in some private plugins. jenkinsci#796 changed signature of this method. Now signature is returned back. See also jenkinsci#817.
Created #818 with alternative fix. Please can you check it. For me both approaches are ok. Just little comment maybe for future development. I would suggest to use approach similar to |
<dependency> | ||
<groupId>com.infradna.tool</groupId> | ||
<artifactId>bridge-method-annotation</artifactId> | ||
<version>1.29</version> | ||
</dependency> |
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 delivered by Jenkins core, so this dependency isn't needed. While in general it is a best practice to declare direct dependencies rather than consuming a transitive dependency without declaring it, with Jenkins plugins we usually recommend the opposite (not declaring a dependency and consuming it transitively via core) in order to avoid bloating JPIs with unused JARs in WEB-INF/lib
and dealing with Enforcer issues.
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.
It wasn't in my classpath when I did the implementation.
The signature change in recent release caused
NoSuchMethodError
in plugins that are compiled against the old code.I've tested it with the PCT and ATH of the affected plugin (cloudbees internal).
Your checklist for this pull request