Fixed bug where Change Desc. was added to initial request publishing
Review Request #6767 — Created Jan. 13, 2015 and submitted
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 … |
 david
|
|
|
Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will … |
david
|
|
|
Unlike most docstrings, we don't include a trailing period in the ones for unit tests, because the test runner will … |
david
|
|
|
You only use summary once. Can you remove the variable definition and just pass the string inline? Alternavitely, since you … |
david
|
JY
- Change Summary:
-
Just some typos / clarifications.
- Description:
-
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 i private unpublished review request is discarded.
~ 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.
- Testing Done:
-
- 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!
-
-
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.
-
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.
-
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.
-
You only use
summaryonce. 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.
JY
- Change Summary:
-
Updating based on David's feedback.
- Commit:
-
31a9f1ac52dbe48dc7b6dcf7c58a477ba8fef18c069231cda9a7884cdfb80bf7ba4fe5dcf977691c
-
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
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