flake8
passed.
JSHint
passed.
Fixed disabled buttons if an error happens updating a review field.
Review Request #10689 — Created Sept. 6, 2019 and updated
The publish button ends up being disabled while the changes made to the
model are being saved, so that the user does not repeatedly click the
publish button. If the request fails due to an error with one of the
fields that were changed, then the publish button does not ever end up
being re-enabled. Now, the model emits an event when an error happens
in the above case, and the view listens for that event being published.
When the view is notified of that event, it re-enables the buttons.
I tested this locally by following the instructions listed on the bug report page (bug #4777). I also ran the JavaScript unit tests.
| Summary | ID | Author |
|---|---|---|
| 380ddd8571c53a98797f281e55fbbd855f6b79d0 | amalik2 |
| Description | From | Last Updated |
|---|---|---|
|
Same comment here as on /r/10688 regarding the format we like for commit descriptions. Also note that you should keep … |
 chipx86
|
|
|
Can you wrap your description at 72 characters? |
 brennie
|
|
|
We can't currently land review requests that have more than one commit (I never got around to writing that before … |
brennie
|
|
|
One small nit and then I'm happy. Markdown is super great in a review request, but becomes the content for … |
chipx86
|
|
|
We prefer single quotes over double quotes when possible (exceptions being when the text itself contains a single quote). |
chipx86
|
|
|
This should also use single quotes. Lines must also be less than 80 characters in length. |
chipx86
|
|
|
Single quotes here as well. |
chipx86
|
-
-
Same comment here as on /r/10688 regarding the format we like for commit descriptions.
Also note that you should keep lines at around the 72 character point, so make sure your editor is set to wrap there.
-
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revision 1) We prefer single quotes over double quotes when possible (exceptions being when the text itself contains a single quote).
-
-
reviewboard/static/rb/js/views/reviewRequestEditorView.es6.js (Diff revision 1) Single quotes here as well.
Description: |
|
|---|
Commits: |
|
|||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 2 (+37 -3) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
Commits: |
|
|||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 3 (+40 -4) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
-
-
-
We can't currently land review requests that have more than one commit (I never got around to writing that before I left
) so can you squash your changes together?Even if we could, we would rather avoid fixup commits since they don't really add anything to the history.
If you're unsure about how to squash them I'm happy to show you.
Description: |
|
||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Testing Done: |
|
Commits: |
|
||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Diff: |
Revision 4 (+48 -4) |
Checks run (2 succeeded)
flake8
passed.
JSHint
passed.
Summary: |
|
||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Description: |
|
-
-
One small nit and then I'm happy.
Markdown is super great in a review request, but becomes the content for the commit message, and Markdown is not a standardized thing there. So the link in Testing Done is going to look strange.
Since the bug report is already listed, and will be included in the commit message, just report the link. You can instead say "... listed on bug #4777."
Testing Done: |
|
|---|