-
-
Notifications
You must be signed in to change notification settings - Fork 105
[JENKINS-26097] Adjust label validation and auto-completion #136
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
Conversation
…ion. This also sets the the delimiter character on the label textbox to a space, so that choosing "bar" after typing "foo && b" doesn't replace the whole thing. (Plugin parent bumped to 4.2 to resolve dependencies of the recent Jenkins incremental.)
|
At first glance, the test failures don't seem to come from these changes. Perhaps compatibility issue with newer Jenkins? |
No. I would think jenkinsci/jenkins-test-harness#210 would have solved all these, as in jenkinsci/workflow-basic-steps-plugin#116 (comment), but you did update the parent here. @dwnusbaum any idea offhand? I can dig into it. |
Best guess is that it has to do with issues fixed in workflow-job 2.33+. This plugin is still using 2.31, so I would try updating dependencies to see if that makes a difference. @Zastai If you are interested, you can try pulling the latest version of https://github.com/jenkinsci/bom into this PR to update everything, or we can handle it in a separate PR. |
jglick
left a comment
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.
Other than test failures, LGTM
src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java
Outdated
Show resolved
Hide resolved
src/main/java/org/jenkinsci/plugins/workflow/support/steps/WorkspaceStepExecution.java
Outdated
Show resolved
Hide resolved
Added a dependency on the junit plugin (1.20); this seems to be required by the InjectedTest from the plugin parent, so it's a bit odd we need to add an explicit dependency here.
|
Looks like the bump to workflow-job 2.33 did it; made the tests pass for me. I did have to add an explicit dependency on junit 1.20 for the InjectedTest (which I think comes from the plugin parent) failed (because it seems to use matrix-project 1.14, which wants junit 1.20, and apparently 1.6 was being pulled in). |
Uses the API being introduced for JENKINS-26097 (jenkinsci/jenkins#4774)
(Also squishes a spotbugs whine.)