Clean up desktop notifications

Review Request #8072 — Created March 22, 2016 and submitted

Information

Review Board
release-3.0.x

Reviewers

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.

chipx86chipx86

Can this just be inline below?

chipx86chipx86

"Update"

daviddavid

Space before {. That's from the original change. Can you go through and make sure there aren't others like this …

chipx86chipx86

Indented 1 too many spaces

daviddavid

Can we wrap the whole conditional in parens?

daviddavid

No indentation for the description. Same below.

chipx86chipx86

"object"

chipx86chipx86

Can we assert the presence of the options we need?

chipx86chipx86

Why the extra variable?

daviddavid

Can we do one per line?

chipx86chipx86

Alphabetical order.

chipx86chipx86

Alphabetical order.

chipx86chipx86
reviewbot
  1. 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
    
    
  2. 
      
david
  1. Typo in summary: "notificaitons"

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

    Indented 1 too many spaces

  3. Can we wrap the whole conditional in parens?

  4. Why the extra variable?

  5. 
      
chipx86
  1. 
      
  2. Comma, not semicolon.

  3. Can this just be inline below?

  4. Space before {.

    That's from the original change. Can you go through and make sure there aren't others like this in there?

  5. No indentation for the description.

    Same below.

  6. Can we assert the presence of the options we need?

  7. Can we do one per line?

  8. reviewboard/staticbundles.py (Diff revision 1)
     
     
     
     

    Alphabetical order.

  9. reviewboard/staticbundles.py (Diff revision 1)
     
     
     
     
     

    Alphabetical order.

  10. 
      
brennie
reviewbot
  1. 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
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to feature/desktop-notifications (e480bc1)
Loading...