Summary

Clean up desktop notifications

Review Request #8072 — Created March 22, 2016 and submitted March 23, 2016, 1:40 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-3.0.x

Bugs

Depends On

~~7660~~

Reviewers

Groups

reviewboard

People

Description

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.

Testing Done

Ran JS tests.

Issues

Description	From	Last Updated
Comma, not semicolon.	chipx86	March 22, 2016, 6:55 p.m.
Can this just be inline below?	chipx86	March 22, 2016, 6:55 p.m.
"Update"	david	March 22, 2016, 6:53 p.m.
Space before {. That's from the original change. Can you go through and make sure there aren't others like this …	chipx86	March 22, 2016, 6:57 p.m.
Indented 1 too many spaces	david	March 22, 2016, 6:54 p.m.
Can we wrap the whole conditional in parens?	david	March 22, 2016, 6:54 p.m.
No indentation for the description. Same below.	chipx86	March 22, 2016, 6:57 p.m.
"object"	chipx86	March 22, 2016, 6:58 p.m.
Can we assert the presence of the options we need?	chipx86	March 22, 2016, 7:01 p.m.
Why the extra variable?	david	March 22, 2016, 6:54 p.m.
Can we do one per line?	chipx86	March 22, 2016, 7:01 p.m.
Alphabetical order.	chipx86	March 22, 2016, 7:02 p.m.
Alphabetical order.	chipx86	March 22, 2016, 7:02 p.m.

Tool: PEP8 Style Checker
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/ui/views/tests/notificationManagerTests.js
    reviewboard/static/rb/js/ui/views/notificationManager.es6.js
    reviewboard/static/rb/js/ui/managers/notificationManagerModel.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js



Tool: Pyflakes
Processed Files:
    reviewboard/staticbundles.py

Ignored Files:
    reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
    reviewboard/static/rb/js/ui/views/tests/notificationManagerTests.js
    reviewboard/static/rb/js/ui/views/notificationManager.es6.js
    reviewboard/static/rb/js/ui/managers/notificationManagerModel.js
    reviewboard/static/rb/js/pages/views/reviewablePageView.js

```
Typo in summary: "notificaitons"
```
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 1)
The issue has been resolved. Show all issues
```
"Update"
```

reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1)

The issue has been resolved. Show all issues

Indented 1 too many spaces

reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we wrap the whole conditional in parens?
```
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Why the extra variable?
```

reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Comma, not semicolon.
```
reviewboard/static/rb/js/pages/views/reviewablePageView.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Can this just be inline below?
```

reviewboard/static/rb/js/ui/views/tests/notificationManagerTests.js (Diff revision 1)

The issue has been resolved. Show all issues

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)
The issue has been resolved. Show all issues
```
No indentation for the description.
Same below.
```
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
"object"
```
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we assert the presence of the options we need?
```
reviewboard/static/rb/js/ui/views/notificationManager.es6.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Can we do one per line?
```
reviewboard/staticbundles.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Alphabetical order.
```
reviewboard/staticbundles.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Alphabetical order.
```

Change Summary:

Address David and Christian's issues

Summary:

Clean up desktop notificaitons

Clean up desktop notifications

Diff:

Revision 2 (+197 -192)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/pages/views/reviewablePageView.js
	reviewboard/static/rb/js/pages/views/tests/reviewablePageViewTests.js
	reviewboard/static/rb/js/ui/managers/notificationManagerModel.js
	reviewboard/static/rb/js/ui/managers/tests/notificationManagerModelTests.js
	reviewboard/static/rb/js/ui/views/notificationManager.es6.js
	reviewboard/static/rb/js/ui/views/tests/notificationManagerTests.es6.js

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

Ship it!

```
Ship It!
```

Status:: Completed
Change Summary:: Pushed to feature/desktop-notifications (e480bc1)