1351749 - mozreview counts number of added lines in virtual commit message file

Status: RESOLVED INVALID

(MozReview Graveyard :: Review Board: DiffViewer, enhancement)

Description

[Andreas Tolfsen ❲:ato❳]

When listing number of added- and removed lines of code for a commit, mozreview also includes the number of lines added for the virtual commit message file.

I just wrote a patch that adds +1 lines, but mozreview claims +5 since the commit message is four lines long.  This is merely a papercut, but it immediately confused me because I thought I had committed more things than I did.

Comment 1

[Andreas Tolfsen ❲:ato❳]

Attachment: screenshot-2017-03-29-183232.png

See attached screen capture for proof.

Comment 2

[Piotr Zalewa [:zalun]]

Attachment: no-commit-msg-in-count.patch

Comment 3

[Piotr Zalewa [:zalun]]

Let's discuss the idea before adding more work on it (tests and such).

Comment 4

[Piotr Zalewa [:zalun]]

Attachment: mozreview: do not count number of ceadded lines in commit message file (bug 1351749)

The `get_diffstats` method is counting all added and removed lines in each FileDiff - including commit msg one. That is misleading.

This patch is modifying the `mozreview.diffviewer.diffstats` method so it subtracts line changes from commit msg FileDiff from the DiffSet's total line count.

Review commit: https://reviewboard.mozilla.org/r/129694/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/129694/

Comment 5

[Piotr Zalewa [:zalun]]

Comment on attachment 8857738 [details]
mozreview: do not count number of ceadded lines in commit message file (bug 1351749)

https://reviewboard.mozilla.org/r/129694/#review132328

::: commit-message-ef32c:1
(Diff revision 1)
> +mozreview: do not count number of ceadded lines in commit message file (bug 1351749) r=davidwalsh

I'll fix the commit msg typo with the first "after review" push

Comment 6

[BMO Automation]

MozReview is now obsolete. Please use Phabricator instead. Closing this bug.