Clean up desktop notifications
Review Request #8072 — Created March 22, 2016 and submitted
This patch fixes the last remaining issues on
https://reviews.reviewboard.org/r/7660/, as well as porting the bulk of
the change to ES6.
Ran JS tests.
Description | From | Last Updated |
---|---|---|
Comma, not semicolon. |
chipx86 | |
Can this just be inline below? |
chipx86 | |
"Update" |
david | |
Space before {. That's from the original change. Can you go through and make sure there aren't others like this … |
chipx86 | |
Indented 1 too many spaces |
david | |
Can we wrap the whole conditional in parens? |
david | |
No indentation for the description. Same below. |
chipx86 | |
"object" |
chipx86 | |
Can we assert the presence of the options we need? |
chipx86 | |
Why the extra variable? |
david | |
Can we do one per line? |
chipx86 | |
Alphabetical order. |
chipx86 | |
Alphabetical order. |
chipx86 |
-
Typo in summary: "notificaitons"
-
-
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1) Indented 1 too many spaces
-
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1) Can we wrap the whole conditional in parens?
-
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1) Why the extra variable?
-
-
-
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 1) Can this just be inline below?
-
reviewboard/static/rb/js/ui/views/tests/notificationManagerTests.js (Diff revision 1) Space before
{
.That's from the original change. Can you go through and make sure there aren't others like this in there?
-
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1) No indentation for the description.
Same below.
-
-
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1) Can we assert the presence of the options we need?
-
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1) Can we do one per line?
-
-
Change Summary:
Address David and Christian's issues
-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/ui/views/tests/notificationManagerTests.es6.js reviewboard/static/rb/js/ui/views/notificationManager.es6.js reviewboard/static/rb/js/ui/managers/notificationManagerModel.js reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js reviewboard/static/rb/js/pages/views/reviewablePageView.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/ui/views/tests/notificationManagerTests.es6.js reviewboard/static/rb/js/ui/views/notificationManager.es6.js reviewboard/static/rb/js/ui/managers/notificationManagerModel.js reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js reviewboard/static/rb/js/pages/views/reviewablePageView.js