-
Notifications
You must be signed in to change notification settings - Fork 398
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
[FIXED JENKINS-35550] Upgrade to Credentials 2.1.0+ API for populating credentials drop-down #154
Conversation
…g credentials drop-down - In two of the cases the context is explicitly `Jenkins.getInstance()` and no user credentials should be considered. - In one case - conversion - then user credentials stores should be included
@@ -105,7 +105,7 @@ | |||
<dependency> | |||
<groupId>org.jenkins-ci.plugins</groupId> | |||
<artifactId>credentials</artifactId> | |||
<version>1.22</version> | |||
<version>2.1.8</version> |
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.
why??
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.
recommended version in the 2.1.x series
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.
So it will require everybody update to latest credentials without alternative?
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.
Yes. if you want to go down as far as 2.1.6 you can... but the diff between 2.1.6 and 2.1.8 is recommended given that I have not seen any issues in 2.1.8 in over 2 weeks
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 saw the prior comment in the credentials plugin wiki which seemed to indicate that 2.1.7 was preferred. I assume from your comment that 2.1.8 has been actively used long enough that it is now the preferred version. Much appreciated to have those comments on the wiki page. It gave me a good reason to think about which credentials version I should use for an upcoming git plugin release.
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.
so 2.1.8 was soaked for 2 weeks now... no new bugs on top of 2.1.7... so I am declaring it the preferred version... there is still some issues that plugins have with the 2.1.7+ versions when there is a plugin that iterates an ExtensionList
before InitMilestone.EXTENSIONS_AUGMENTED
and can therefore see partially initialized plugins... but that is a problem with the plugins doing bold things, not the credentials plugin ;-)
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.
You shouldn't require the latest if there is no reason in APIs. Git 3.0 was so cool that i was forced to revert it to 2.x and hopefully nobody enforced latest version. So revert to latest possible according to APIs.
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
will merge and release it at friday (if no one will review it too) |
@KostyaSha github now provides ability to "Require changes" if you wait to fix smth after review. Also - this fix requires 2.1+ api, so why not to use latest stable release? |
I provided real life example. Now there is no any change to use non latest version. |
See JENKINS-35550
Jenkins.getInstance()
and no user credentials should be considered.@reviewbybees @jenkinsci/code-reviewers
This change is