Update the dvcs branch for ES6
Review Request #8056 — Created March 12, 2016 and submitted
This patch updates the
RB.DiffCommitmodel and
RB.DiffCommitIndexViewview to use ES6. As well, the doc comments
have been updated to align with our style guide.
Manually tested that the diff commit index still worked.
| Description | From | Last Updated |
|---|---|---|
|
This should probably just use a defaults object instead of a method. |
 david
|
|
|
This will iterate over non-own properties too. How about: for (let property of Object.keys(this.attributes.lineCounts)) { |
david
|
|
|
This can use the method() sugar. |
david
|
|
|
How about: for (let [i, diffCommit] of Array.entries(this.collection.models)) { |
david
|
|
|
Can you move this up above the "tr" definition? This seems like it doesn't fit in with the rest of … |
david
|
|
|
Template string? |
david
|
|
|
Template string? |
david
|
|
|
const? |
david
|
|
|
Can you align the - with "offset"? |
david
|
|
|
Alignment? |
david
|
|
|
This should be indented 2 more spaces. |
david
|
|
|
Should be indented 2 more spaces. |
david
|
|
|
Ooh, even better: return Object.values(this.get('lineCounts')) .reduce((a, b) => a + b); |
david
|
|
|
return this.collection.any( model => model.get('historyEntrySymbol') !== ' '); |
david
|
|
|
Can be const. |
david
|
-
-
reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js (Diff revision 1) This should probably just use a defaults object instead of a method.
-
reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js (Diff revision 1) This will iterate over non-own properties too. How about:
for (let property of Object.keys(this.attributes.lineCounts)) { -
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 1) This can use the
method()sugar. -
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 1) How about:
for (let [i, diffCommit] of Array.entries(this.collection.models)) { -
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 1) Can you move this up above the "tr" definition? This seems like it doesn't fit in with the rest of the code around it.
-
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 1) Template string?
-
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 1) Template string?
-
-
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 1) Can you align the - with "offset"?
-
-
Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js
-
-
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revisions 1 - 2) This should be indented 2 more spaces.
-
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revisions 1 - 2) Should be indented 2 more spaces.
-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js
-
A couple more suggestions to tighten things up:
-
reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js (Diff revision 3) Ooh, even better:
return Object.values(this.get('lineCounts')) .reduce((a, b) => a + b); -
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 3) return this.collection.any( model => model.get('historyEntrySymbol') !== ' '); -
reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js (Diff revision 3) Can be const.
-
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js
Tool: PEP8 Style Checker Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js Tool: Pyflakes Processed Files: reviewboard/staticbundles.py Ignored Files: reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.es6.js reviewboard/static/rb/js/diffviewer/views/diffCommitIndexView.es6.js reviewboard/static/rb/js/diffviewer/models/diffCommitModel.js