Fixed bug where Change Desc. was added to initial request publishing

Review Request #6767 — Created Jan. 13, 2015 and submitted

Information

Review Board
master
069231c...

Reviewers

This is a fix for the bug I introduced in the fix to 3465.

With this fix:
- Change descriptions are no longer published on initial review request publishings.
- Discarding no longer modifies the fields in the change description (aside from status, which it always did)

The refactored code pretty much changes the state prior to the shipped fix of 3465, and extracts the "copying over from draft to review request" part to a separate function. That function is then called if a private unpublished review request is discarded.

  • Ran existing unit tests & passed
  • Modified previous unit tests that I wrote to reflect the "copying over" part and not the change description. Passed.
  • Added a test to make sure change descriptions are no longer published on initial review request publishings.
  • Also manual tests of the above!
Description From Last Updated

Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will …

daviddavid

Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will …

daviddavid

Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will …

daviddavid

You only use summary once. Can you remove the variable definition and just pass the string inline? Alternavitely, since you …

daviddavid
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/tests.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/models/review_request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/tests.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/models/review_request.py
    
    
  2. 
      
JY
david
  1. 
      
  2. reviewboard/reviews/tests.py (Diff revision 1)
     
     
    Show all issues

    Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will add an ellipsis ('...') after this string in the output.

  3. reviewboard/reviews/tests.py (Diff revision 1)
     
     
    Show all issues

    Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will add an ellipsis ('...') after this string in the output.

  4. reviewboard/reviews/tests.py (Diff revision 1)
     
     
    Show all issues

    Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will add an ellipsis ('...') after this string in the output.

  5. reviewboard/reviews/tests.py (Diff revision 1)
     
     
     
     
     
     
    Show all issues

    You only use summary once. Can you remove the variable definition and just pass the string inline? Alternavitely, since you don't seem to care what the summary is, you could skip it entirely and just let it use the default.

  6. 
      
JY
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/reviews/tests.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/models/review_request.py
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/reviews/tests.py
        reviewboard/reviews/models/review_request_draft.py
        reviewboard/reviews/models/review_request.py
    
    
  2. 
      
chipx86
  1. Ship It!
  2. 
      
JY
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.0.x (60bdb97)