Add parent diff support to the pre-commit section of "New Review Request".
Review Request #6117 — Created July 18, 2014 and submitted
In general, the new "New Review Request" page works very well for people. One
feature that was missed by a small number of users was the ability to specify a
parent diff. This change adds back that feature.If the given diff fails due to missing files or revisions, the user is shown
the error, but additionally given the option of uploading a parent diff. This
keeps the streamlined workflow for the general case but makes it possible for
the complex one.While I was in here, I also plumbed through errors when creating the review
request or diff to display. I don't think we've ever hit these errors in
practice, due to the diff validation step (I had to artificially hack the code
to trigger them).
- Uploaded a plain diff via both DnD and the select button and saw it create
the review request. - Uploaded a diff which required a parent diff and saw the expected error
message with new form. - Uploaded diff + parent diff using both DnD and the select button, and saw it
create the review request. - Uploaded a broken diff and saw the right error.
- Uploaded a diff and parent diff which still required another parent to
completely resolve all changes and saw an error. - Checked that "Start over" worked on both error pages.
- Ran jshint
- Ran js-tests
| Description | From | Last Updated |
|---|---|---|
|
Can we reverse these? Everywhere else in the codebase, I've been ordering the variables so that initialized ones are before … |
 chipx86
|
|
|
We should start moving toward IDs using _ instead of - for new code, since we've been moving other code … |
chipx86
|
|
|
Space after the comma. |
chipx86
|
|
|
I know this was already here, but this should be done through events: {} |
chipx86
|
|
|
This error string is computed twice. We should maybe just pull that out somewhere. |
chipx86
|
|
|
This error string is computed twice. We should maybe just pull that out somewhere. |
chipx86
|
Testing Done: |
|
|---|
-
-
reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js (Diff revision 1) Can we reverse these? Everywhere else in the codebase, I've been ordering the variables so that initialized ones are before uninitialized ones.
-
reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js (Diff revision 1) We should start moving toward IDs using
_instead of-for new code, since we've been moving other code to that style.Same with other new IDs in this file.
-
reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js (Diff revision 1) Space after the comma.
-
reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js (Diff revision 1) I know this was already here, but this should be done through
events: {} -
reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js (Diff revision 1) This error string is computed twice. We should maybe just pull that out somewhere.
-
reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js (Diff revision 1) This error string is computed twice. We should maybe just pull that out somewhere.
Commit: |
|
||||
|---|---|---|---|---|---|
Diff: |
Revision 2 (+105 -48) |
-
Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js reviewboard/static/rb/css/newReviewRequest.less Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js reviewboard/static/rb/css/newReviewRequest.less
Tool: Pyflakes Ignored Files: reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js reviewboard/static/rb/css/newReviewRequest.less Tool: PEP8 Style Checker Ignored Files: reviewboard/static/rb/js/newReviewRequest/models/preCommitModel.js reviewboard/static/rb/js/newReviewRequest/views/preCommitView.js reviewboard/static/rb/css/newReviewRequest.less