Skip to content
This repository has been archived by the owner on Dec 15, 2021. It is now read-only.

[JENKINS-30122] AbstractSynchronousNonBlockingStepExecution neglected to pick up the build’s authentication #192

Conversation

jglick
Copy link
Member

@jglick jglick commented Aug 25, 2015

JENKINS-30122

@reviewbybees esp. @amuniz

@ghost
Copy link

ghost commented Aug 25, 2015

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.

@@ -61,21 +78,4 @@ private static synchronized ExecutorService getExecutorService() {
return executorService;
}

private static class StepRunner implements Runnable {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was it needed? I see it clearer by having the run logic inside this private inner class. It would suffice to pass the auth object in the constructor and store it in a local field.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seemed easier and clearer to me to use a final local variable used close to its declaration, and let javac take care of the rest.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no accounting for taste... but I'll try to follow your preferred notation in the future in order to have a uniform style across the project.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not worry about it, I was just editing this and found it clearer here.

@amuniz
Copy link
Member

amuniz commented Aug 25, 2015

🐝

@amuniz
Copy link
Member

amuniz commented Aug 25, 2015

I must wait until the build finishes to set the bee... reverting to a 🐛 until the findbugs issue is solved.

@amuniz
Copy link
Member

amuniz commented Aug 26, 2015

🐝

jglick added a commit that referenced this pull request Aug 26, 2015
…pExecution-authentication-JENKINS-30122

[JENKINS-30122] AbstractSynchronousNonBlockingStepExecution neglected to pick up the build’s authentication
@jglick jglick merged commit bcc1b24 into jenkinsci:master Aug 26, 2015
@jglick jglick deleted the AbstractSynchronousNonBlockingStepExecution-authentication-JENKINS-30122 branch August 26, 2015 13:33
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants