From b53c1f6748ac5a38c7268c3d0e4c731c3cbfe047 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 26 Aug 2014 14:51:33 +0900 Subject: [PATCH 1/2] Add example of range comment in REST API documentation Bug: Issue 2861 Change-Id: Id9db8fd7a56178f469b1ed17d1cac14c78bfff79 --- Documentation/rest-api-changes.txt | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 3385378801..0d0d27bbe1 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -1545,6 +1545,16 @@ link:#review-input[ReviewInput] entity. { "line": 49, "message": "[nit] s/conrtol/control" + }, + { + "range": { + "start_line": 50, + "start_character": 0, + "end_line": 55, + "end_character": 20 + }, + "line": 55, + "message": "Incorrect indentation" } ] } From 4a159a1edfe8a66672736120774571ae420245d2 Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Tue, 26 Aug 2014 15:45:14 +0900 Subject: [PATCH 2/2] Replace "line" with "end_line" when range is given in inline comment When a range comment is given in the review, the caller must specify the "range" entity with "start_line" and "end_line" values, and also the "line" value. The documentation simply says that the "line" value 'should equal the end line of the range', but the implementation does not validate this. If "line" is not specified, the comment is added as a file comment regardless of what "start_line" and "end_line" values are given in the range. This can be confusing for users. Change it so that when a "range" is given, the "line" value is not necessary, and if given at all will be overridden with the value from the "range" Bug: Issue 2861 Change-Id: I5d0d31c392937b1955572781375fc48982ea7664 --- Documentation/rest-api-changes.txt | 3 +-- .../main/java/com/google/gerrit/server/change/PostReview.java | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt index 0d0d27bbe1..4c599f141c 100644 --- a/Documentation/rest-api-changes.txt +++ b/Documentation/rest-api-changes.txt @@ -1553,7 +1553,6 @@ link:#review-input[ReviewInput] entity. "end_line": 55, "end_character": 20 }, - "line": 55, "message": "Incorrect indentation" } ] @@ -2900,7 +2899,7 @@ If not set, the default is `REVISION`. The number of the line for which the comment should be added. + `0` if it is a file comment. + If neither line nor range is set, a file comment is added. + -If range is set, this should equal the end line of the range. +If range is set, this value is ignored in favor of the `end_line` of the range. |`range` |optional| The range of the comment as a link:#comment-range[CommentRange] entity. 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 25c63b5d11..022538796c 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 @@ -339,6 +339,7 @@ public class PostReview implements RestModifyView c.range.startCharacter, c.range.endLine, c.range.endCharacter)); + e.setLine(c.range.endLine); } ups.add(e); }