diff options
author | Ismo Haataja <ismo.haataja@digia.com> | 2014-08-15 12:55:24 +0300 |
---|---|---|
committer | Oswald Buddenhagen <oswald.buddenhagen@qt.io> | 2017-04-27 17:57:36 +0200 |
commit | 9b90d93a5c25dce64e30c37dca44693d49a1a6e4 (patch) | |
tree | 951f3b1c9ba1f3a1176d16c699925d9ec7819d6e | |
parent | 92ae224f7f21e543397c7e880a9ea3f6536cb68c (diff) |
Drop extra review note when doing certain operations via SSHv2.7-56-based-v2
Abandon via SSH always created two comments for a change, 'review note' and
'abandon comment'. Same happened with defer and restore operations. Example
command and output below:
'ssh demosrv gerrit review --abandon -m "Example comment" sha1'
Review note: 'Patch Set 3: Example comment'
Abandon comment: 'Abandoned Example comment'
This is not what one would expect to happen. One comment is enough and
'abandon comment' would be the preferred one. Just like it happens via UI.
This is what this patch does. 'Review note' is dropped and just 'abandon
comment' is kept. Same goes for defer and restore.
'ssh demosrv gerrit review --abandon -m "Comment fixed" sha1'
Abandon comment: 'Abandoned Comment fixed'
'Review note' is necessary just when labels (review scores) are applied.
So with review labels applied, this patch changes nothing:
'ssh demosrv gerrit review --abandon -m "Comment" sha1 -l Sanity-Review=0'
Review note: 'Patch Set 3: -Sanity-Review Comment'
Abandon comment: 'Abandoned'
In this case 'abandon comment' is simply 'abandoned' and has no scores neither
given message included.
============
upstream issue https://bugs.chromium.org/p/gerrit/issues/detail?id=1721
dave pursehouse had a stab at it for 2.6 in
a65b695ee5232309f01304a1694dd5eacd04caf4, but evidently failed.
i didn't find an equivalent upstream change in the more recent history,
but i take no bets.
============
Task-number: QTQAINFRA-886
Change-Id: I790caa91df603748139881b123da0072fea6dd74
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@theqtcompany.com>
Reviewed-by: Ismo Haataja <ismo.haataja@digia.com>
-rw-r--r-- | gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java | 5 | ||||
-rw-r--r-- | gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java | 11 |
2 files changed, 13 insertions, 3 deletions
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java index 1fe9ef1b30..2c8c93f81b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java @@ -88,6 +88,7 @@ public class PostReview implements RestModifyView<RevisionResource, Input> { public NotifyHandling notify = NotifyHandling.ALL; public boolean changeReviewable; + public boolean dontAddReviewNote = false; } public static enum DraftHandling { @@ -157,7 +158,9 @@ public class PostReview implements RestModifyView<RevisionResource, Input> { boolean dirty = false; dirty |= insertComments(revision, input.comments, input.drafts); dirty |= updateLabels(revision, input.labels); - dirty |= insertMessage(revision, input.message); + if (!input.dontAddReviewNote) { + dirty |= insertMessage(revision, input.message); + } if (dirty) { db.changes().update(Collections.singleton(change)); db.commit(); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index a26dfd7259..37f75e744a 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -287,8 +287,15 @@ public class ReviewCommand extends SshCommand { // If review labels are being applied, the comment will be included // on the review note. We don't need to add it again on the abandon, // defer or restore comment. - if (!review.labels.isEmpty() && (abandonChange || deferChange || restoreChange)) { - changeComment = null; + if (abandonChange || deferChange || restoreChange) { + if (!review.labels.isEmpty()) { + changeComment = null; + } + else { + // If no review labels applied, this drops the review note. + // The comment is visible in the abandon, defer or restore comment. + review.dontAddReviewNote = true; + } } try { |