Skip to content
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

Also specifiy a tag when using the SSH API #368

Merged
merged 1 commit into from Aug 23, 2018

Conversation

Projects
None yet
2 participants
@sschuberth
Copy link
Contributor

commented Jul 2, 2018

As a follow-up to [1], also add a tag by default when using the SSH API,
not only when using the REST API.

[1] #317

@@ -26,6 +26,7 @@

import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeBasedEvent;
import com.sonymobile.tools.gerrit.gerritevents.workers.rest.AbstractRestCommandJob;
import com.sonyericsson.hudson.plugins.gerrit.trigger.config.Constants;

This comment has been minimized.

Copy link
@rsandell

rsandell Jul 10, 2018

Member

unused import? Should have been caught by checkstyle?

This comment has been minimized.

Copy link
@sschuberth

sschuberth Jul 10, 2018

Author Contributor

No, the import is not unused, it just was not needed previously as Constants was in the same package. Now that I moved that class, the import is needed. Similar below.

This comment has been minimized.

Copy link
@rsandell

rsandell Jul 10, 2018

Member

Oh, I missed that the class was moved. Why did you do that?

This comment has been minimized.

Copy link
@sschuberth

sschuberth Jul 10, 2018

Author Contributor

Because it was in a rest-specific package, but in fact the constant in there i snot REST-specific (anymore).

PS: I'll resolve the conflict now.

@@ -26,6 +26,7 @@

import com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeBasedEvent;
import com.sonymobile.tools.gerrit.gerritevents.workers.rest.AbstractRestCommandJob;
import com.sonyericsson.hudson.plugins.gerrit.trigger.config.Constants;

This comment has been minimized.

Copy link
@rsandell

rsandell Jul 10, 2018

Member

unused import?

This comment has been minimized.

Copy link
@sschuberth

sschuberth Jul 18, 2018

Author Contributor

See above, no, it's required now.

Also specifiy a tag when using the SSH API
As a follow-up to [1], also add a tag by default when using the SSH API,
not only when using the REST API.

[1] #317

@sschuberth sschuberth force-pushed the sschuberth:ssh-api-tag branch from 6cc3f2e to 303eadf Jul 10, 2018

@sschuberth

This comment has been minimized.

Copy link
Contributor Author

commented Jul 27, 2018

@rsandell I believe to have addressed all of your comments.

@rsandell rsandell merged commit 66f2f7e into jenkinsci:master Aug 23, 2018

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details

@sschuberth sschuberth deleted the sschuberth:ssh-api-tag branch Aug 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.