Summary

Port the diffviewer to ES6.

Review Request #8070 — Created March 21, 2016 and discarded July 13, 2016, 1:25 p.m.

Information

Owner

brennie

Repository

Review Board

Branch

release-2.6.x

Bugs

Depends On

Reviewers

Groups

reviewboard

People

Description

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

Testing Done

Ran JS tests.

Issues

Description	From	Last Updated
/**	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
Returns?	david	March 22, 2016, 12:31 p.m.
Clamp. Also needs Args/Returns.	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
Can you move these definitions down to where they're needed (at a glance, iconView can be defined much lower, and …	david	March 22, 2016, 12:31 p.m.
The IIFE isn't necessary in this file.	david	March 22, 2016, 12:31 p.m.
Render. Also needs Returns.	david	March 22, 2016, 12:31 p.m.
Probably slightly more idiomatic to use $.each().	david	March 22, 2016, 12:31 p.m.
This is only used once, so you could move it into the conditional.	david	March 22, 2016, 12:31 p.m.
Could be one statement with ?:	david	March 22, 2016, 12:31 p.m.
/**, Create. Also needs Args/Returns.	david	March 22, 2016, 12:31 p.m.
/**, Place. Also needs Args/Returns.	david	March 22, 2016, 12:31 p.m.
let/const?	david	March 22, 2016, 12:31 p.m.
/**, Place.	david	March 22, 2016, 12:31 p.m.
This can be cleaned up a lot now: const hiddenCommentBlockViews = this._hiddenCommentBlockViews; this._hiddenCommentBlockViews = []; let prevBeginRowIndex; for (let commentBlockView …	david	March 22, 2016, 12:31 p.m.
Doc comment?	david	March 22, 2016, 12:31 p.m.
Can clean up with let/const and _.groupBy	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
let/const?	david	March 22, 2016, 12:31 p.m.
/**, Args	david	March 22, 2016, 12:31 p.m.
const?	david	March 22, 2016, 12:31 p.m.
let/const?	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
let/const?	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
let/const?	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
/**, Args.	david	March 22, 2016, 12:31 p.m.
/**	david	March 22, 2016, 12:31 p.m.
/**, Args.	david	March 22, 2016, 12:31 p.m.
/**, Args.	david	March 22, 2016, 12:31 p.m.
/**, Args.	david	March 22, 2016, 12:31 p.m.
const	david	March 22, 2016, 12:31 p.m.
/**, Args.	david	March 22, 2016, 12:31 p.m.
const.	david	March 22, 2016, 12:31 p.m.
const	david	March 22, 2016, 12:31 p.m.
const.	david	March 22, 2016, 12:31 p.m.
const.	david	March 22, 2016, 12:31 p.m.
This file hasn't been converted very much...	david	March 22, 2016, 12:31 p.m.
Two /*s here.	david	March 22, 2016, 12:31 p.m.

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

Diff:

Revision 2 (+692 -515)

Show changes

	reviewboard/staticbundles.py
	reviewboard/static/rb/js/diffviewer/collections/diffFileCollection.js
	reviewboard/static/rb/js/diffviewer/models/diffCommentBlockModel.js
	reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.js
	reviewboard/static/rb/js/diffviewer/models/diffCommentsHintModel.es6.js
	reviewboard/static/rb/js/diffviewer/models/diffFileModel.js
	reviewboard/static/rb/js/diffviewer/models/diffReviewableModel.js
	reviewboard/static/rb/js/diffviewer/models/diffRevisionModel.js
	21 more

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

Fix it!

reviewboard/static/rb/js/diffviewer/models/paginationModel.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/models/paginationModel.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/views/diffCommentBlockView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/views/diffCommentsHintView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
Returns?
```
reviewboard/static/rb/js/diffviewer/views/diffComplexityIconView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
Clamp. Also needs Args/Returns.
```
reviewboard/static/rb/js/diffviewer/views/diffFileIndexView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```

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

An issue was opened. Show all issues

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

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
The IIFE isn't necessary in this file.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
Render. Also needs Returns.
```

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

An issue was opened. Show all issues

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

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
This is only used once, so you could move it into the conditional.
```

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

An issue was opened. Show all issues

Could be one statement with ?:

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Create. Also needs Args/Returns.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Place. Also needs Args/Returns.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
let/const?
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Place.
```

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

An issue was opened. Show all issues

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);
}

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
Doc comment?
```

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

An issue was opened. Show all issues

Can clean up with let/const and _.groupBy

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
let/const?
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Args
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
const?
```

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

An issue was opened. Show all issues

let/const?

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```

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

An issue was opened. Show all issues

let/const?

reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
let/const?
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Args.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Args.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Args.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Args.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
const
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
/**, Args.
```
reviewboard/static/rb/js/diffviewer/views/diffReviewableView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
const.
```
reviewboard/static/rb/js/diffviewer/views/diffRevisionSelectorView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
const
```
reviewboard/static/rb/js/diffviewer/views/diffRevisionSelectorView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
const.
```
reviewboard/static/rb/js/diffviewer/views/diffRevisionSelectorView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
const.
```
reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
This file hasn't been converted very much...
```
reviewboard/static/rb/js/pages/views/diffViewerPageView.es6.js (Diff revision 2)
An issue was opened. Show all issues
```
Two /*s here.
```

Status:: Discarded