From 9b90d93a5c25dce64e30c37dca44693d49a1a6e4 Mon Sep 17 00:00:00 2001 From: Ismo Haataja Date: Fri, 15 Aug 2014 12:55:24 +0300 Subject: Drop extra review note when doing certain operations via SSH 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 Reviewed-by: Ismo Haataja --- .../main/java/com/google/gerrit/server/change/PostReview.java | 5 ++++- .../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 { 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 { 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 { -- cgit v1.2.3