Fix Default reviewers not added to review request for committed change
Review Request #7691 — Created Oct. 10, 2015 and submitted
Create a review request for a committed change via the "New Review Request" page. Default reviewer is not shown. The season of this bug is because the code of adding default reviewer is only called when posting change via RBtools.
To solve this, add default reviewers when creating a review request from a committed change
- Create a review request from "New Review Request" web UI, the default reviewer was shown.
- Run all the unit tests, and pass all the tests.
| Description | From | Last Updated |
|---|---|---|
|
Col: 1 W293 blank line contains whitespace |
 reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 77 W291 trailing whitespace |
reviewbot
|
|
|
Undo this change. |
 brennie
|
|
|
Undo this change. |
brennie
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
|
|
Col: 1 W293 blank line contains whitespace |
reviewbot
|
-
Tool: Pyflakes Processed Files: reviewboard/reviews/managers.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/managers.py
-
Hey Xutong! Thanks for the patch. Before this can land, we'll probably want a more comprehensive Summary, Description and Testing Done.
Please see: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/
-
Your summary is much improved but you still need more extensive description and testing done fields. Please also add the bug number into the bugs field.
- Description:
-
~ add function review_request.add_default_reviewers(), fix bug 3900
~ Fix the problem in reviewboard/reviews/managers.py
+ When a review request has been created and save. Default reviewer did not been added to the review request. Therefore, default groups and people did not show. Problem solved after adding add_default_reviewers() after review_request.save(). Then, after review request been saved, defaul_reviewer can be appended.
- Bugs:
- Testing Done:
-
+ First, locating where the add_default_reviewers function is more important. Then, figuring out that the add_default_reviewers function has not been alled. So, finding where the add_default_reviewers is key to solve the bug.
+ After located where the update_from_commit_id() function been called, I added the add_default_reviewers() blow, but it does not fix the bug. Becasue the request review does not been saved and default_reviewer can not been depended in database somewhere. Therefore, adding the add_default_reviewers() under review_request.save(), then the problem solved.
-
I have only two minor nitpicks for the code and a few general comments about your description and testing done.
Your summary doesn't need to mention what files were edited, that's what the diff is for :)
The description should be a high-level overview of what the issue was and how it was fixed.Also, your testing done should describe how you tested the code works, e.g.
- Ran unit tests - Created a post-commit review request and saw that the appropriate default reviewers were added.You also have some grammar issues in your description and should probably give this a read: https://www.reviewboard.org/docs/codebase/dev/writing-good-descriptions/
-
-
-
Tool: Pyflakes Processed Files: reviewboard/reviews/managers.py Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/managers.py
- Description:
-
~ Fix the problem in reviewboard/reviews/managers.py
~ A new review request is created without any default reviewer attached. Therefore, default groups and people did not show. Problem solved after adding add_default_reviewers() after review_request.save(). Then, after review request been saved, defaul_reviewer can be appended.
- When a review request has been created and save. Default reviewer did not been added to the review request. Therefore, default groups and people did not show. Problem solved after adding add_default_reviewers() after review_request.save(). Then, after review request been saved, defaul_reviewer can be appended.
- Description:
-
~ A new review request is created without any default reviewer attached. Therefore, default groups and people did not show. Problem solved after adding add_default_reviewers() after review_request.save(). Then, after review request been saved, defaul_reviewer can be appended.
~ A new review request is created without any default reviewer attached. Therefore, default groups and people did not show. The problem is fixed by add default reviewer after a new review request created and saved.
- Testing Done:
-
~ First, locating where the add_default_reviewers function is more important. Then, figuring out that the add_default_reviewers function has not been alled. So, finding where the add_default_reviewers is key to solve the bug.
~ First, test by trying to reproduce the bug, click a New Review Request and then select already commited change, the default reviewers and default groups have been shown. Then, run all unit tests proved for review board to make sure all other functions works as well.
- After located where the update_from_commit_id() function been called, I added the add_default_reviewers() blow, but it does not fix the bug. Becasue the request review does not been saved and default_reviewer can not been depended in database somewhere. Therefore, adding the add_default_reviewers() under review_request.save(), then the problem solved.
- Description:
-
~ A new review request is created without any default reviewer attached. Therefore, default groups and people did not show. The problem is fixed by add default reviewer after a new review request created and saved.
~ A new review request is created without any default reviewer attached. Therefore, default groups and people did not show. The problem is fixed by add default reviewer after a new review request created and saved. Then the default reviewers can be appended after a new review request has been saved.
-
Your description and testing done are somewhat difficult to follow. I'd suggest being a little less verbose in the description, and making testing done a set of bullet points. For example:
When creating a review request for a committed change via the "New Review Request" page, default reviewers were not added to it. This was happening because the code to add those reviewers was only called when posting changes from RBTools. To fix this, in the review request manager, we now explicitly add default reviewers when creating a review request from a committed change.Testing done: - Created a new review request from the web UI and saw that default reviewers were correctly added. - Ran unit tests.
- Description:
-
~ A new review request is created without any default reviewer attached. Therefore, default groups and people did not show. The problem is fixed by add default reviewer after a new review request created and saved. Then the default reviewers can be appended after a new review request has been saved.
~ Create a review request for a commited change via the "New Review Request" page. Default reviewer is not shown. The season of this bug is because the code of adding defaul reviewer is only called when posting change via RBtools.
+ To solve this, add default reviewers when creating a review request from a committed change
- Testing Done:
-
~ First, test by trying to reproduce the bug, click a New Review Request and then select already commited change, the default reviewers and default groups have been shown. Then, run all unit tests proved for review board to make sure all other functions works as well.
~ - Create a review request from "New Review Request" web UI, the default reviewer was shown.
+ - Run all the unit tests, and pass all the tests.
- Description:
-
~ Create a review request for a commited change via the "New Review Request" page. Default reviewer is not shown. The season of this bug is because the code of adding defaul reviewer is only called when posting change via RBtools.
~ Create a review request for a committed change via the "New Review Request" page. Default reviewer is not shown. The season of this bug is because the code of adding default reviewer is only called when posting change via RBtools.
To solve this, add default reviewers when creating a review request from a committed change
Tool: PEP8 Style Checker Processed Files: reviewboard/reviews/managers.py Tool: Pyflakes Processed Files: reviewboard/reviews/managers.py