[JENKINS-44112] - Enable WorkDir in JNLPLauncher #2945
Conversation
oleg-nenashev
added
needs-more-reviews
| @@ -86,6 +122,20 @@ public void launch(SlaveComputer computer, TaskListener listener) { | |||
| */ | |||
| public static /*almost final*/ Descriptor<ComputerLauncher> DESCRIPTOR; | |||
|
|
|||
| /** | |||
| * gets | |||
| @Issue("JENKINS-44112") | ||
| public void testNoWorkDirMigration() throws Exception { | ||
| Computer computer = j.jenkins.getComputer("Foo"); | ||
| Assert.assertThat(computer, instanceOf(SlaveComputer.class)); |
| } | ||
|
|
||
| StringBuilder bldr = new StringBuilder(); | ||
| bldr.append("-workDir \""); |
There was a problem hiding this comment.
Is this going to work with windows? I am not 100% sure but I feel like that backslash is for the start of a file in windows rather than a special character announcement
There was a problem hiding this comment.
It is. Windows also needs escaping
| @@ -0,0 +1,4 @@ | |||
| <div> | |||
| If defined, Remoting will fail the startup if the target work directory is missing. | |||
There was a problem hiding this comment.
"Remoting will fail at startup" I think rather than "Remoting will fail the startup"
|
LGTM as well with a few small nitpicky things |
oleg-nenashev
removed
the
work-in-progress
|
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. |
|
@reviewbybees @rysteboe @alexanderrtaylor Ready for the final review |
|
@reviewbybees done. I kindly ask @jenkinsci/code-reviewers to take a look, because I would like to get this change integrated into the weekly to have it in the next LTS baseline |
oleg-nenashev
removed
the
needs-more-reviews
|
Merged the change in order to have it in the weekly. |
See JENKINS-44112. Since Jenkins 2.57 we can enable Remoting Work Directories.
@DataBoundSetterto get it supported in child classes automagically?UI samples
Config:
Main page:
Proposed changelog entries
Submitter checklist
* Use the
Internal:prefix if the change has no user-visible impact (API, test frameworks, etc.)Desired reviewers
@reviewbybees
@batmat since it collides with #2775