[JENKINS-34131] Upgrade to plugin parent pom 2.6 #5
Conversation
|
Thank you for this pull request! Please check this document for how the Jenkins project handles pull requests. |
armfergom
changed the title
|
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. |
| @@ -207,7 +209,8 @@ public void launch(final SlaveComputer computer, final TaskListener listener) th | |||
| SWbemServices services = WMI.connect(session, name); | |||
|
|
|||
|
|
|||
| String path = computer.getNode().getRemoteFS(); | |||
| Slave node = computer.getNode(); | |||
| String path = node != null ? node.getRemoteFS() : ""; | |||
There was a problem hiding this comment.
Specially review this. If node == null, I am not so sure what the behaviour should be. Maybe throw an error better than what's in the PR ATM?
| @@ -207,7 +208,8 @@ public void launch(final SlaveComputer computer, final TaskListener listener) th | |||
| SWbemServices services = WMI.connect(session, name); | |||
|
|
|||
|
|
|||
| String path = computer.getNode().getRemoteFS(); | |||
| Slave node = computer.getNode(); | |||
| String path = node != null ? node.getRemoteFS() : ""; | |||
| if (path.indexOf(':')==-1) throw new IOException("Remote file system root path of the slave needs to be absolute: "+path); | |||
There was a problem hiding this comment.
(currently this will get thrown but with a confusing message)
|
🐝
Hardly. I split it out of core, that is all. |
|
Thanks for the review. Comments addressed. |
| if (node != null) { | ||
| String id = generateServiceId(node.getRemoteFS()); | ||
| Win32Service slaveService = services.getService(id); | ||
| if(slaveService!=null) { |
There was a problem hiding this comment.
Code style, you need spaces before and after !=
|
LGTM 🐝 |
|
🐝 |
|
@reviewbybees done |
|
This pull request has completed our internal processes and we now respectfully request the maintainers of this repository to consider our proposal contained within this pull request for merging. |
@reviewbybees