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
This commit is contained in:
David Pursehouse 2014-08-26 15:45:14 +09:00
parent b53c1f6748
commit 4a159a1edf
2 changed files with 2 additions and 2 deletions

View File

@ -1553,7 +1553,6 @@ link:#review-input[ReviewInput] entity.
"end_line": 55, "end_line": 55,
"end_character": 20 "end_character": 20
}, },
"line": 55,
"message": "Incorrect indentation" "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. + The number of the line for which the comment should be added. +
`0` if it is a file comment. + `0` if it is a file comment. +
If neither line nor range is set, a file comment is added. + 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| |`range` |optional|
The range of the comment as a link:#comment-range[CommentRange] The range of the comment as a link:#comment-range[CommentRange]
entity. entity.

View File

@ -339,6 +339,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
c.range.startCharacter, c.range.startCharacter,
c.range.endLine, c.range.endLine,
c.range.endCharacter)); c.range.endCharacter));
e.setLine(c.range.endLine);
} }
ups.add(e); ups.add(e);
} }