Fish Trophy

brennie got a fish trophy!

Add resources for interacting with DiffCommits

Review Request #9889 — Created May 2, 2018 and updated

brennie
Review Board
release-4.0.x
9883
9905
dd3d6d7...
reviewboard

The DraftDiffCommitResource allows new commits to be uploaded to a
review request draft with history support and the DiffCommitResource
allows fetching information about created commits.

Ran unit tests.

  • 2
  • 0
  • 60
  • 0
  • 62
Description From Last Updated
F401 'reviewboard.diffviewer.models.DiffSet' imported but unused reviewbot
F841 local variable 'diffset' is assigned to but never used reviewbot
Checks run (1 failed, 1 succeeded)
flake8 failed.
JSHint passed.

flake8

brennie
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
brennie
brennie
chipx86
  1. 
      
  2. reviewboard/reviews/forms.py (Diff revision 10)
     
     
     
     

    This isn't in the function signature.

  3. reviewboard/reviews/forms.py (Diff revision 10)
     
     

    Missing a docstring.

  4. reviewboard/reviews/forms.py (Diff revision 10)
     
     
     
     
     

    Too many blank lines.

  5. reviewboard/webapi/resources/__init__.py (Diff revision 10)
     
     
     

    The and .. or form was used in the past due to lack of the inline conditional support in earlier versions of Python. We should just use the inline form here.

  6. We shouldn't use literals in summaries.

    Same below.

  7. reviewboard/webapi/resources/diff_commit.py (Diff revision 10)
     
     
     

    You can just use ObjectDoesNotExist here.

  8. reviewboard/webapi/resources/diff_commit.py (Diff revision 10)
     
     
     
     
     
     
     
     
     
     

    Everything is indented 1 space in the body.

  9. reviewboard/webapi/resources/diff_commit.py (Diff revision 10)
     
     
     
     
     
     
     
     
     
     

    Why can't we just tack on ?commit-id=... onto the URL? Are we dealing with dynamic content in the link that could interfere with that?

  10. Let's explicitly compare obj against None, since the former's gotten us into trouble before with valid but falsy-evaluating objects.

  11. reviewboard/webapi/resources/diff_commit.py (Diff revision 10)
     
     
     
     

    Can you pass these params as keyword arguments, to help with readability?

  12. Trailing comma is turning this into a tuple.

    Same with other descriptions.

  13. Are we guaranteed that this is going to be compatible all the time with the args we put in?

    I'm tempted to say let's not provide the URL, but rather mention the resource we care about (maybe referencing the link name on the appropriate resource). Callers should not be hard-coding URLs anyway.

  14. Can you make an explicit logger?

  15. reviewboard/webapi/resources/draft_diff_commit.py (Diff revision 10)
     
     
     
     
     

    Same comments as above.

  16. reviewboard/webapi/resources/review_request_draft.py (Diff revision 10)
     
     
     
     
     
     

    Incomplete docstring.

  17. reviewboard/webapi/tests/mimetypes.py (Diff revision 10)
     
     
     
     
     
     
     
     
     

    Needs 2 blank lines between sections.

  18. reviewboard/webapi/tests/test_diff.py (Diff revision 10)
     
     

    Can you use a keyword argument for True? I have no idea what this is from reading this code.

    This applies to all occurrences in the change.

  19. reviewboard/webapi/tests/test_diff.py (Diff revision 10)
     
     
     
     
     
     
     

    Too many blank lines.

  20. reviewboard/webapi/tests/test_diff.py (Diff revision 10)
     
     
     

    These can be combined.

  21. reviewboard/webapi/tests/test_diff.py (Diff revision 10)
     
     
     

    These too.

  22. reviewboard/webapi/tests/test_diff_commit.py (Diff revision 10)
     
     
     
     
     

    Need tests for requesting patch content.

  23. reviewboard/webapi/tests/test_draft_diff.py (Diff revision 10)
     
     
     

    Swap these.

  24. Blank line between these.

  25. 
      
brennie
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
brennie
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

Loading...