Port the diffviewer to ES6.

Review Request #8070 — Created March 21, 2016 and discarded

Information

Review Board
release-2.6.x

Reviewers

This change ports the js/diffviewer and the diff viewer page's view
and model to ES6.

Ran JS tests.

Description From Last Updated

/**

daviddavid

/**

daviddavid

/**

daviddavid

/**

daviddavid

Returns?

daviddavid

Clamp. Also needs Args/Returns.

daviddavid

/**

daviddavid

Can you move these definitions down to where they're needed (at a glance, iconView can be defined much lower, and …

daviddavid

The IIFE isn't necessary in this file.

daviddavid

Render. Also needs Returns.

daviddavid

Probably slightly more idiomatic to use $.each().

daviddavid

This is only used once, so you could move it into the conditional.

daviddavid

Could be one statement with ?:

daviddavid

/**, Create. Also needs Args/Returns.

daviddavid

/**, Place. Also needs Args/Returns.

daviddavid

let/const?

daviddavid

/**, Place.

daviddavid

This can be cleaned up a lot now: const hiddenCommentBlockViews = this._hiddenCommentBlockViews; this._hiddenCommentBlockViews = []; let prevBeginRowIndex; for (let commentBlockView …

daviddavid

Doc comment?

daviddavid

Can clean up with let/const and _.groupBy

daviddavid

/**

daviddavid

let/const?

daviddavid

/**, Args

daviddavid

const?

daviddavid

let/const?

daviddavid

/**

daviddavid

let/const?

daviddavid

/**

daviddavid

let/const?

daviddavid

/**

daviddavid

/**, Args.

daviddavid

/**

daviddavid

/**, Args.

daviddavid

/**, Args.

daviddavid

/**, Args.

daviddavid

const

daviddavid

/**, Args.

daviddavid

const.

daviddavid

const

daviddavid

const.

daviddavid

const.

daviddavid

This file hasn't been converted very much...

daviddavid

Two /*s here.

daviddavid
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.js
        reviewboard/static/rb/js/diffviewer/views/chunkHighlighterView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentBlockModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffRevisionModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.es6.js
        reviewboard/static/rb/js/views/imageReviewableView.js
        reviewboard/static/rb/js/diffviewer/models/tests/paginationModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffFileModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/paginationModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.es6.js
        reviewboard/static/rb/js/diffviewer/collections/diffFileCollection.js
        reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffFileModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffComplexityIconView.es6.js
        reviewboard/static/rb/js/pages/models/diffViewerPageModel.js
        reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionSelectorView.es6.js
        reviewboard/static/rb/js/views/textBasedReviewableView.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.js
        reviewboard/static/rb/js/views/dummyReviewableView.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.js
        reviewboard/static/rb/js/diffviewer/views/chunkHighlighterView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentBlockModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffRevisionModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.es6.js
        reviewboard/static/rb/js/views/imageReviewableView.js
        reviewboard/static/rb/js/diffviewer/models/tests/paginationModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffFileModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/paginationModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.es6.js
        reviewboard/static/rb/js/diffviewer/collections/diffFileCollection.js
        reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffFileModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffComplexityIconView.es6.js
        reviewboard/static/rb/js/pages/models/diffViewerPageModel.js
        reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionSelectorView.es6.js
        reviewboard/static/rb/js/views/textBasedReviewableView.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.js
        reviewboard/static/rb/js/views/dummyReviewableView.js
    
    
  2. 
      
brennie
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.js
        reviewboard/static/rb/js/diffviewer/views/chunkHighlighterView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentBlockModel.es6.js
        reviewboard/static/rb/js/pages/models/diffViewerPageModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffRevisionModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.es6.js
        reviewboard/static/rb/js/views/imageReviewableView.js
        reviewboard/static/rb/js/diffviewer/models/tests/paginationModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffFileModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/paginationModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.es6.js
        reviewboard/static/rb/js/diffviewer/collections/diffFileCollection.js
        reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffFileModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffComplexityIconView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionSelectorView.es6.js
        reviewboard/static/rb/js/views/textBasedReviewableView.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.js
        reviewboard/static/rb/js/views/dummyReviewableView.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/staticbundles.py
    
    Ignored Files:
        reviewboard/static/rb/js/diffviewer/models/tests/diffReviewableModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.js
        reviewboard/static/rb/js/diffviewer/views/chunkHighlighterView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentBlockModel.es6.js
        reviewboard/static/rb/js/pages/models/diffViewerPageModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffRevisionModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.js
        reviewboard/static/rb/js/diffviewer/views/paginationView.es6.js
        reviewboard/static/rb/js/views/imageReviewableView.js
        reviewboard/static/rb/js/diffviewer/models/tests/paginationModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffFileModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/paginationModel.es6.js
        reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.es6.js
        reviewboard/static/rb/js/diffviewer/collections/diffFileCollection.js
        reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionLabelView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.es6.js
        reviewboard/static/rb/js/diffviewer/models/tests/diffFileModelTests.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffComplexityIconView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.es6.js
        reviewboard/static/rb/js/diffviewer/views/diffRevisionSelectorView.es6.js
        reviewboard/static/rb/js/views/textBasedReviewableView.js
        reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.js
        reviewboard/static/rb/js/views/dummyReviewableView.js
    
    
  2. 
      
david
  1. 
      
  2. Clamp. Also needs Args/Returns.

  3. reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.es6.js (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Can you move these definitions down to where they're needed (at a glance, iconView can be defined much lower, and probably others too).

  4. The IIFE isn't necessary in this file.

  5. Render. Also needs Returns.

  6. reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    Probably slightly more idiomatic to use $.each().

  7. This is only used once, so you could move it into the conditional.

  8. Could be one statement with ?:

  9. /**, Create. Also needs Args/Returns.

  10. /**, Place. Also needs Args/Returns.

  11. reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
     
     
     
     
     
     
     
     
     
     

    This can be cleaned up a lot now:

    const hiddenCommentBlockViews = this._hiddenCommentBlockViews;
    
    this._hiddenCommentBlockViews = [];
    
    let prevBeginRowIndex;
    
    for (let commentBlockView of hiddenCommentBlockViews) {
        prevBeginRowIndex = this._placeCommentBlockView(
            commentBlockView, prevBeginRowIndex);
    }
    
  12. reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     

    Can clean up with let/const and _.groupBy

  13. This file hasn't been converted very much...

  14. 
      
brennie
Review request changed

Status: Discarded

Loading...