[JENKINS-34923] #2395
[JENKINS-34923] #2395
Conversation
Signed-off-by: Nicolas De Loof <[email protected]>
oleg-nenashev
added
the
needs-more-reviews
|
It would be useful if the PR includes a summary of the change and justification. Or at least a link to JIRA. Otherwise reviewers have to rad the full conversation log |
|
@oleg-nenashev the Jira issue is in the PR title |
|
see related code use on https://github.com/jenkinsci/one-shot-executor-plugin/tree/JENKINS-34923 |
|
@ndeloof thanks! |
|
any feedback on this ? Would like to get this merged for 2.9 |
|
👍 |
| @@ -204,6 +204,10 @@ public Slave getNode() { | |||
| } | |||
| } | |||
|
|
|||
| public TaskListener getListener() { | |||
There was a problem hiding this comment.
should really have a mininimal bit of Javadoc as it is a new public method in core.
it requires at least a @since TODO fix when merging
There was a problem hiding this comment.
/**
* accessor method for SlaveComputer's {@link TaskListener}
* @return the SlaveComputer's {@link TaskListener}
*/
Would this really be useful ?
There was a problem hiding this comment.
it is a start. will it always return a valid object - can it return null - when was the method introduced?
There was a problem hiding this comment.
|
code looks fine to me, but the javadoc needs work. |
| @@ -207,6 +216,21 @@ public static void fireCompleted(Run r, @Nonnull TaskListener listener) { | |||
| } | |||
|
|
|||
| /** | |||
| * Fires the {@link #onInitialize(Run)} event. | |||
| */ | |||
| public static void fireInitialize(Run r) { | |||
There was a problem hiding this comment.
shouldn't something be calling this? (links to my prior question - what does it mean for a Run to be initialized - when does this happen) - are you expecting all slave types to have to fire this?
|
🐝 |
oleg-nenashev
removed
the
needs-more-reviews
read https://issues.jenkins-ci.org/browse/JENKINS-34923 for justification
proposed change has minimal impact on core and delegates log encoding consistency to plugin relying on new Run event to do some pre-computation.
Signed-off-by: Nicolas De Loof [email protected]