Prevent review requests created with history from adding regular diffs

Review Request #9852 — Created April 3, 2018 and updated

brennie
Review Board
release-4.0.x
9757, 9899
9883
ebe75b5...
reviewboard

We now have a notion of a review request that is created with history
support. A review request created with this flag cannot have a regular
diff attached to it. Instead, an empty DiffSet must be created and
commits attached to that. (Support for which is coming in another
patch.)

This option is now specified at review request creation time and cannot
be changed once the review request is created (i.e., after save() is
called and the primary key has been assigned). It can be specified via
a new field on the review request resources which defaults to the old
(i.e., no history) behaviour.

Ran unit tests.

  • 0
  • 0
  • 39
  • 0
  • 39
Description From Last Updated
chipx86
  1. 
      
  2. reviewboard/reviews/forms.py (Diff revision 1)
     
     

    Needs to be the full path.

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

    Missing ugettext().

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

    Is this going to be seen by end users ever? If so, we shouldn't be talking DiffSets and DiffCommits, and need to rethink what we're telling users.

  5. reviewboard/reviews/managers.py (Diff revision 1)
     
     

    Needs to be the full module path.

  6. reviewboard/reviews/managers.py (Diff revision 1)
     
     
     

    This will be just a tad more clear with "... cannot both be set to True".

  7. Missing ending paren.

  8. reviewboard/reviews/models/review_request.py (Diff revision 1)
     
     
     
     
     
     

    This would probably be better as:

    return (self.extra_data and
            self.extra_data.get(..., False))
    
  9. reviewboard/reviews/tests/test_forms.py (Diff revision 1)
     
     

    Can you also have this check the resulting errors?

  10. This should check the message.

  11. reviewboard/reviews/tests/test_review_request.py (Diff revision 1)
     
     
     
     

    Can you group those create_ flags together?

  12. Spaces around *.

  13. reviewboard/reviews/tests/test_review_request.py (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Should this be two tests?

  14. This should check the error.

  15. reviewboard/testing/testcase.py (Diff revision 1)
     
     
     
     

    Should have ~ at the beginning.

    These are annoyingly long, and means this must be kept up-to-date. Maybe just have this refer to the list of status on the class (with a reference to it).

    (Also, these would be :py:attr, not :py:data).

  16. reviewboard/testing/testcase.py (Diff revision 1)
     
     
     
     

    We can probably just talk about publishing rather than have a long line for the function reference.

  17. reviewboard/testing/testcase.py (Diff revision 1)
     
     

    Needs to be the full reference.

  18. Can you have this say explicitly that "This field cannot be set if create_with_history is set" ?

  19. And similar here.

  20. Can you put the create_ arguments together?

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

    Can you have this check the "reason" message?

  22. reviewboard/webapi/tests/test_diff.py (Diff revision 1)
     
     
     
     

    Two blank lines.

  23. Can you have this check the "reason" message?

  24. This can fit on one line.

  25. This doesn't need to be indented. item_rsp['extra_data'] can fit right after the assertEqual(.

  26. 
      
brennie
Review request changed

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
brennie
brennie
Review request changed

Change Summary:

Hide DVCS fields when the feature is disabled. Do not allow creation of DVCS review requests when feature is disabled.

Commit:

-7e0d7f02b6229b04bfa6847e85a327d5f485aac0
+bc63b1a7f058833236c9eeda538ab31e0dd15897

Diff:

Revision 6 (+573 -22)

Show changes

Checks run (1 failed, 1 succeeded)

flake8 failed.
JSHint passed.

flake8

brennie
brennie
brennie
david
  1. 
      
  2. Should be ValueError

  3. reviewboard/webapi/resources/diff.py (Diff revision 9)
     
     

    typo: mulitple

  4. reviewboard/webapi/tests/test_diff.py (Diff revision 9)
     
     

    typo: mulitple

  5. typo: mulitple

  6. 
      
brennie
brennie
david
  1. Ship It!
  2. 
      
chipx86
  1. 
      
  2. reviewboard/reviews/forms.py (Diff revision 11)
     
     
     
     
     

    Missing localization.

  3. reviewboard/testing/testcase.py (Diff revision 11)
     
     

    "STATUSES"

  4. reviewboard/webapi/tests/test_review_request.py (Diff revision 11)
     
     
     
     
     

    Extra blank line.

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

    "associated"

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

    This needs to explicitly use ugettext().

  4. reviewboard/webapi/resources/diff.py (Diff revision 14)
     
     

    Missing a trailing comma. (So is the one above.)

  5. Should be assertFalse

  6. 
      
brennie
Review request changed
Loading...