Closed
Bug 1124764
Opened 9 years ago
Closed 9 years ago
Change dumpreview output to YAML
Categories
(MozReview Graveyard :: General, defect)
MozReview Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: gps, Assigned: gps)
Details
Attachments
(2 files, 1 obsolete file)
See description.
Assignee | ||
Comment 1•9 years ago
|
||
/r/2849 - testing: convert output of rbmanage dumpreview to YAML (bug 1124764) Pull down this commit: hg pull review -r 90d44913aec9304189d129007c3d50efb6e8789e
Assignee | ||
Comment 2•9 years ago
|
||
Comment on attachment 8553212 [details] MozReview Request: bz://1124764/gps /r/2849 - testing: convert output of rbmanage dumpreview to YAML (bug 1124764) /r/2855 - testing: dump review info from review requests Pull down these commits: hg pull review -r 4227e666c95c9b51077d12cb3ae11b9e0564d488
Assignee | ||
Comment 3•9 years ago
|
||
Comment on attachment 8553212 [details] MozReview Request: bz://1124764/gps /r/2849 - testing: convert output of rbmanage dumpreview to YAML (bug 1124764) /r/2855 - testing: dump review info from review requests Pull down these commits: hg pull review -r 4227e666c95c9b51077d12cb3ae11b9e0564d488
Attachment #8553212 -
Flags: review?(smacleod)
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8553212 [details] MozReview Request: bz://1124764/gps /r/2849 - testing: convert output of rbmanage dumpreview to YAML (bug 1124764) /r/2855 - testing: dump review info from review requests Pull down these commits: hg pull review -r 4e54a6ac80e650a703fdcdf39a9fc3444bf08691
Comment 5•9 years ago
|
||
https://reviewboard.mozilla.org/r/2849/#review2139 I gave the first few tests a really throrough inspection to make sure things looked correct and then mostly just skimmed the rest. This looks good though, I like how the output is much less ambiguous. ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 2) > import yaml > > -# TODO Use YAML. > +# Teach yaml how to format OrderedDict. nit: toss another blank here. ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 2) > import os > import sys > > +from collections import OrderedDict shouldn't we have these all grouped together? ::: hgext/reviewboard/tests/test-commits-deleted-no-obsolescence.t (Diff revision 2) > - p2rb.discard_on_publish_rids: ["6"] > + p2rb.commits: '[["c5b850e249510046906bcb24f774635c4521a4a9", "2"], ["905ad211ecc6f024e1f0ffdbe084dd06cf28ae1c", > + "3"], ["68fdf92dbf149ab8afb8295a76b79fb82a9629b1", "4"], ["53b32d356f20f6730c14ec62c3706eba7e68e078", > + "5"], ["f466ed1de51670e583e11deb2f1022a342b52ccd", "6"]]' Ya, if you know to expect this type of wrapping it's okay. It looks to be a sane predictable algorithm for the wrapping... I hope it doesn't cause problems with strange long lines.
Comment 6•9 years ago
|
||
https://reviewboard.mozilla.org/r/2855/#review2141 ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 2) > + for review in rr.get_reviews(review_request_id=rr.id): This will break if there are over 25 reviews, since it will paginate the response (probably will never happen, but still). You can pass `max_results=200` here to increase the default but 200 is your limit. That'd probably be more than sufficient for tests (well, 25 probably is in all honesty). The other option is to also print out `review.total_results`, which will be how many exist even if it is greater than the page size, and then truncate displaying at the first 25 etc. Feel free to just make the decision to drop this issue, just wanted you to at least think about it. ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 2) > + for comment in review.get_diff_comments(): Same thing with respect to the pagination and result limit here. It's actually a lot more likely diffcomments could overflow. ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 2) > + #interfilediff = comment.get_interfilediff() I'm torn on whether to include this. Without we can't test comments on interdiffs, but we probably never will anyways.
Updated•9 years ago
|
Attachment #8553212 -
Flags: review?(smacleod) → review+
Comment 7•9 years ago
|
||
Comment on attachment 8553212 [details] MozReview Request: bz://1124764/gps https://reviewboard.mozilla.org/r/2847/#review2143 Ship It!
Assignee | ||
Comment 8•9 years ago
|
||
Comment on attachment 8553212 [details] MozReview Request: bz://1124764/gps /r/2849 - testing: convert output of rbmanage dumpreview to YAML (bug 1124764); r=smacleod /r/2855 - testing: dump review info from review requests Pull down these commits: hg pull review -r 6f0f109f12c713a7236985e033481e02ddf1e0e9
Attachment #8553212 -
Flags: review+ → review?(smacleod)
Comment 9•9 years ago
|
||
https://reviewboard.mozilla.org/r/2849/#review2145 Always look on the bright side of life. I analyzed your Python changes and found 4 errors. The following files were examined: testing/vcttesting/reviewboard/mach_commands.py ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 3) > - print(' Public: %s' % r.public) > - if r.bugs_closed: > +yaml.add_representer(OrderedDict, ordered_dict_presenter, > + Dumper=yaml.SafeDumper) E128: continuation line under-indented for visual indent Continuation lines indentation. Continuation lines should align wrapped elements either vertically using Python's implicit line joining inside parentheses, brackets and braces, or using a hanging indent. When using a hanging indent these considerations should be applied: - there should be no arguments on the first line, and - further indentation should be used to clearly distinguish itself as a continuation line. Okay: a = (\n) E123: a = (\n ) Okay: a = (\n 42) E121: a = (\n 42) E122: a = (\n42) E123: a = (\n 42\n ) E124: a = (24,\n 42\n) E125: if (\n b):\n pass E126: a = (\n 42) E127: a = (24,\n 42) E128: a = (24,\n 42) E129: if (a or\n b):\n pass E131: a = (\n 42\n 24) ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 3) > - print(' Commit ID: %s' % r.commit_id) > +def _serialize_text(s): E302: expected 2 blank lines, found 1 Separate top-level function and class definitions with two blank lines. Method definitions inside a class are separated by a single blank line. Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations). Use blank lines in functions, sparingly, to indicate logical sections. Okay: def a():\n pass\n\n\ndef b():\n pass Okay: def a():\n pass\n\n\n# Foo\n# Bar\n\ndef b():\n pass E301: class Foo:\n b = 0\n def bar():\n pass E302: def a():\n pass\n\ndef b(n):\n pass E303: def a():\n pass\n\n\n\ndef b(n):\n pass E303: def a():\n\n\n\n pass E304: @decorator\n\ndef a():\n pass ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 3) > - for k, v in sorted(r.extra_data.iteritems()): > +def serialize_review_requests(rr): E302: expected 2 blank lines, found 1 Separate top-level function and class definitions with two blank lines. Method definitions inside a class are separated by a single blank line. Extra blank lines may be used (sparingly) to separate groups of related functions. Blank lines may be omitted between a bunch of related one-liners (e.g. a set of dummy implementations). Use blank lines in functions, sparingly, to indicate logical sections. Okay: def a():\n pass\n\n\ndef b():\n pass Okay: def a():\n pass\n\n\n# Foo\n# Bar\n\ndef b():\n pass E301: class Foo:\n b = 0\n def bar():\n pass E302: def a():\n pass\n\ndef b(n):\n pass E303: def a():\n pass\n\n\n\ndef b(n):\n pass E303: def a():\n\n\n\n pass E304: @decorator\n\ndef a():\n pass
Comment 10•9 years ago
|
||
https://reviewboard.mozilla.org/r/2855/#review2147 Always look on the bright side of life. I analyzed your Python changes and found 1 errors. The following files were examined: testing/vcttesting/reviewboard/mach_commands.py ::: testing/vcttesting/reviewboard/mach_commands.py (Diff revision 3) > + #interfilediff = comment.get_interfilediff() E265: block comment should start with '# ' Separate inline comments by at least two spaces. An inline comment is a comment on the same line as a statement. Inline comments should be separated by at least two spaces from the statement. They should start with a # and a single space. Each line of a block comment starts with a # and a single space (unless it is indented text inside the comment). Okay: x = x + 1 # Increment x Okay: x = x + 1 # Increment x Okay: # Block comment E261: x = x + 1 # Increment x E262: x = x + 1 #Increment x E262: x = x + 1 # Increment x E265: #Block comment
Assignee | ||
Comment 11•9 years ago
|
||
Comment on attachment 8553212 [details] MozReview Request: bz://1124764/gps /r/2849 - testing: convert output of rbmanage dumpreview to YAML (bug 1124764); r=smacleod /r/2855 - testing: dump review info from review requests Pull down these commits: hg pull review -r 8775ff61a1ed1636aab7040bcd19cb8a3ad1e63f
Comment 12•9 years ago
|
||
https://reviewboard.mozilla.org/r/2849/#review2149 Always look on the bright side of life. I analyzed your Python changes and found 1 errors. The following files were examined: testing/vcttesting/reviewboard/mach_commands.py
Comment 13•9 years ago
|
||
https://reviewboard.mozilla.org/r/2855/#review2151 And now for something completely different. Congratulations, there we no Python static analysis issues with this patch! The following files were examined: testing/vcttesting/reviewboard/mach_commands.py
Assignee | ||
Comment 14•9 years ago
|
||
https://hg.mozilla.org/hgcustom/version-control-tools/rev/f8a98dd80c63
Assignee | ||
Updated•9 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Updated•9 years ago
|
Attachment #8553212 -
Flags: review?(smacleod)
Assignee | ||
Comment 15•9 years ago
|
||
Attachment #8553212 -
Attachment is obsolete: true
Assignee | ||
Comment 16•9 years ago
|
||
Assignee | ||
Comment 17•9 years ago
|
||
Updated•8 years ago
|
Product: Developer Services → MozReview
You need to log in
before you can comment on or make changes to this bug.
Description
•