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

Set tag for review comments on REST #317

Merged
merged 1 commit into from Jun 16, 2017

Conversation

Projects
None yet
2 participants
@orgads
Copy link
Contributor

commented Jun 15, 2017

This allows to filter out these messages on Gerrit 2.14 UI

[JENKINS-43904]

@orgads

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2017

Do you prefer to have it configurable? I'm not sure there's a reason...

@@ -116,7 +116,8 @@ protected ReviewInput createReview() {
}
}

return new ReviewInput(message, scoredLabels, commentedFiles).setNotify(notificationLevel);
return new ReviewInput(message, scoredLabels, commentedFiles).setNotify(notificationLevel)
.setTag("jenkins-gerrit-trigger");

This comment has been minimized.

Copy link
@rsandell

rsandell Jun 15, 2017

Member

I would prefer it is a constant somewhere instead of a literal.

This comment has been minimized.

Copy link
@orgads

orgads Jun 15, 2017

Author Contributor

Can you suggest where?

This comment has been minimized.

Copy link
@orgads

orgads Jun 15, 2017

Author Contributor

Done

@rsandell

This comment has been minimized.

Copy link
Member

commented Jun 15, 2017

We can start by having it as a constant, and if someone wants it configurable they are welcome to contribute that feature ;)

Have you tried this on an older Gerrit that doesn't support tag to see if it still works?
If it doesn't we will nee to add some version checks. see https://github.com/jenkinsci/gerrit-trigger-plugin/blob/master/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/version/GerritVersionChecker.java#L44

@orgads

This comment has been minimized.

Copy link
Contributor Author

commented Jun 15, 2017

@rsandell commented on Jun 15, 2017, 12:25 PM GMT+3:

Have you tried this on an older Gerrit that doesn't support tag to see if it still works?

Tried on https://codereview.qt-project.org which has Gerrit 2.7 (manually, posted a review with tag by REST). It works fine.

Set tag for review comments on REST
This allows to filter out these messages on Gerrit 2.14 UI

[JENKINS-43904]

@orgads orgads force-pushed the orgads:set-tag branch from 8406d62 to c6437c8 Jun 15, 2017

@rsandell rsandell merged commit 682ac6e into jenkinsci:master Jun 16, 2017

1 check passed

Jenkins This pull request looks good
Details

@orgads orgads deleted the orgads:set-tag branch Jun 18, 2017

@orgads

This comment has been minimized.

Copy link
Contributor Author

commented Jun 27, 2017

Can you please release it?

@rsandell

This comment has been minimized.

Copy link
Member

commented Jul 3, 2017

Sorry, got lost in my pile of TODOs

Released 2.24.0 a few minutes ago

sschuberth added a commit to sschuberth/gerrit-trigger-plugin that referenced this pull request Jul 2, 2018

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] jenkinsci#317

sschuberth added a commit to sschuberth/gerrit-trigger-plugin that referenced this pull request Jul 10, 2018

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] jenkinsci#317
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.