Closed
Bug 1286017
Opened 9 years ago
Closed 9 years ago
Include diffstat information in the ReviewRequestSummary resource
Categories
(MozReview Graveyard :: Review Board: Extension, defect)
MozReview Graveyard
Review Board: Extension
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: smacleod, Assigned: smacleod)
References
Details
Attachments
(5 files)
58 bytes,
text/x-review-board-request
|
glob
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
glob
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
glob
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
glob
:
review+
|
Details |
58 bytes,
text/x-review-board-request
|
mcote
:
review+
|
Details |
This will allow us to display the diffstat information in the BMO MozReview section.
Assignee | ||
Comment 1•9 years ago
|
||
This commit ignores extra information Review Board has, such as
information on how many lines were replaced, rather than just deleted
and inserted. This is done to keep things simple as the extra
information isn't always present and I haven't thought of a good way
to conditionally pass things along.
This also makes consuming this new data much simpler as two numbers
which will always be present are used. These numbers are also pretty
self explanatory.
In the future we might want to expand this, but we can always keep
the current numbers we provide and just add new information.
Review commit: https://reviewboard.mozilla.org/r/63536/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63536/
Attachment #8769846 -
Flags: review?(glob)
Attachment #8769847 -
Flags: review?(glob)
Attachment #8769848 -
Flags: review?(glob)
Attachment #8769849 -
Flags: review?(glob)
Assignee | ||
Comment 2•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/63538/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63538/
Assignee | ||
Comment 3•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/63540/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63540/
Assignee | ||
Comment 4•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/63542/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/63542/
Comment on attachment 8769846 [details]
mozreview: add functions for retrieving diffstat information (Bug 1286017).
https://reviewboard.mozilla.org/r/63536/#review60562
::: pylib/mozreview/mozreview/diffviewer/__init__.py:8
(Diff revision 1)
> +
> +def diffstats(diffset):
> + """Return a dictionary of diffstat information for a diffset."""
> + counts = diffset.get_total_line_counts()
> +
> + # TODO: Take into account the non raw counts review board might
please file a bug for this TODO
::: pylib/mozreview/mozreview/diffviewer/__init__.py:28
(Diff revision 1)
> + review_request = review_request.get_review_request()
> +
> + if rev is None:
> + # If the user is the submitter we might want to use the draft diffset.
> + draft = review_request.get_draft(user=user)
> + diffset = ((draft and draft.diffset) or
i'd like to see a testcase that ensures the counts are present and correct for submitter + draft changes.
Attachment #8769846 -
Flags: review?(glob) → review+
Attachment #8769847 -
Flags: review?(glob) → review+
Comment on attachment 8769847 [details]
mozreview: cleanup identation in summary example (Bug 1286017).
https://reviewboard.mozilla.org/r/63538/#review60564
Comment on attachment 8769848 [details]
mozreview: add diffstat information to the ReviewRequestSummaryResource (Bug 1286017).
https://reviewboard.mozilla.org/r/63540/#review60566
Attachment #8769848 -
Flags: review?(glob) → review+
Comment on attachment 8769849 [details]
mozreview: add diffstat information to test output (Bug 1286017).
https://reviewboard.mozilla.org/r/63542/#review60568
Attachment #8769849 -
Flags: review?(glob) → review+
Assignee | ||
Comment 9•9 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/65024/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/65024/
Attachment #8772125 -
Flags: review?(glob)
Assignee | ||
Comment 10•9 years ago
|
||
Comment on attachment 8769846 [details]
mozreview: add functions for retrieving diffstat information (Bug 1286017).
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63536/diff/1-2/
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8769847 [details]
mozreview: cleanup identation in summary example (Bug 1286017).
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63538/diff/1-2/
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8769848 [details]
mozreview: add diffstat information to the ReviewRequestSummaryResource (Bug 1286017).
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63540/diff/1-2/
Assignee | ||
Comment 13•9 years ago
|
||
Comment on attachment 8769849 [details]
mozreview: add diffstat information to test output (Bug 1286017).
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63542/diff/1-2/
Comment 14•9 years ago
|
||
https://reviewboard.mozilla.org/r/63540/#review61994
::: pylib/mozreview/mozreview/resources/review_request_summary.py:213
(Diff revision 2)
> {
> 'commit': 'ece2029d013af68f9f32aa0a6199fcb2201d5aae',
> + 'diff': {
> + 'insert': 15,
> + 'delete': 20
> + }
Missing a comma here.
Assignee | ||
Comment 15•9 years ago
|
||
https://reviewboard.mozilla.org/r/63540/#review61994
> Missing a comma here.
Nope, this is example json and following the convention of the rest of the payload. Dropping :D
Assignee | ||
Comment 16•9 years ago
|
||
https://reviewboard.mozilla.org/r/63540/#review61994
> Nope, this is example json and following the convention of the rest of the payload. Dropping :D
woops I'm silly, will fix :D
Assignee | ||
Comment 17•9 years ago
|
||
Comment on attachment 8769848 [details]
mozreview: add diffstat information to the ReviewRequestSummaryResource (Bug 1286017).
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63540/diff/2-3/
Assignee | ||
Comment 18•9 years ago
|
||
Comment on attachment 8769849 [details]
mozreview: add diffstat information to test output (Bug 1286017).
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/63542/diff/2-3/
Assignee | ||
Comment 19•9 years ago
|
||
Comment on attachment 8772125 [details]
mozreview: add testing for diffstats on drafts (Bug 1286017).
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65024/diff/1-2/
Comment 20•9 years ago
|
||
Comment on attachment 8772125 [details]
mozreview: add testing for diffstats on drafts (Bug 1286017).
https://reviewboard.mozilla.org/r/65024/#review62000
smacleod asked me to look at this. lgtm
Attachment #8772125 -
Flags: review+
Updated•9 years ago
|
Attachment #8772125 -
Flags: review?(glob)
Comment 21•9 years ago
|
||
Pushed by smacleod@mozilla.com:
https://hg.mozilla.org/hgcustom/version-control-tools/rev/08364a5147e6
mozreview: add functions for retrieving diffstat information . r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/d4a6879dbf0b
mozreview: cleanup identation in summary example . r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/d3c0c1ca5572
mozreview: add diffstat information to the ReviewRequestSummaryResource . r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/4222094d9397
mozreview: add diffstat information to test output . r=glob
https://hg.mozilla.org/hgcustom/version-control-tools/rev/cd617ebf2dd7
mozreview: add testing for diffstats on drafts . r=mcote
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•