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

Fix backward compatibility of PullRequestSCMRevision #818

Conversation

andrey-fomin
Copy link
Contributor

Method getPull is used in some private plugins.
#796 changed signature of this method.

Now signature is returned back.
See also #817.

Your checklist for this pull request

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side) and not your master branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or in Jenkins JIRA
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Did you provide a test-case? That demonstrates feature works or fixes the issue.

Method `getPull` is used in some private plugins.
jenkinsci#796 changed signature of this method.

Now signature is returned back.
See also jenkinsci#817.
@rsandell
Copy link
Member

Thanks for the alternative approach, the small risk with this approach is that potentially some places might already have adapted to the new signature that is already in a couple of releases. And now it would be changed back again. So I think my approach is slightly safer.

@basil
Copy link
Member

basil commented Feb 28, 2024

Yes safer in the short-term, but less maintainable in the long term since the bridge method injector is some complicated magic and only used in a handful of plugins. No strong preference, but I can see the wisdom in biting the bullet now with this approach (and dealing with any fallout) in order to avoid the long-term dependency on the bridge method injector.

@andrey-fomin
Copy link
Contributor Author

#817 was merges. So closing this one.

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.

3 participants