Convert ReviewRequestEditorModel to ES6.
Review Request #9147 — Created Aug. 23, 2017 and submitted
Another simple conversion of code that I'm working on.
Ran js-tests.
Description | From | Last Updated |
---|---|---|
Missing period. |
brennie | |
Should we instead be building an array and calling .join('')? Also: ` and "${value}"` |
brennie | |
`"${value}"` |
brennie | |
This should probably still pass in the function directly. No sense creating a wrapper function that does nothing but call … |
chipx86 | |
.bind(this) is implied when using this.listenTo(). |
chipx86 | |
Same here. |
chipx86 | |
Same here. |
chipx86 | |
I liked using options much better, because it allows room for expansion. I'm not sure getDraftField should assume that useExtraData … |
chipx86 | |
Why the direct comparison to true? |
chipx86 | |
This could use template literals to compose the string. |
chipx86 | |
Same here. |
chipx86 | |
Sentence casing. |
chipx86 | |
This could be one line. |
chipx86 |
-
-
-
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revision 1) Should we instead be building an array and calling
.join('')
?Also:
` and "${value}"`
-
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 2 (+157 -124) |
Checks run (2 succeeded)
-
-
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revision 2) This should probably still pass in the function directly. No sense creating a wrapper function that does nothing but call the function that
listenTo
would call itself. -
-
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revision 2) I liked using
options
much better, because it allows room for expansion. I'm not suregetDraftField
should assume thatuseExtraData
is the only field-related option that's going to come up (or that it should be so prominent). Can we go back tooptions
? -
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revision 2) Why the direct comparison to
true
? -
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revision 2) This could use template literals to compose the string.
-
-
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revision 2) This could be one line.
Commit: |
|
||||
---|---|---|---|---|---|
Diff: |
Revision 3 (+159 -121) |
Checks run (2 succeeded)
-
-
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revisions 2 - 3) .bind(this)
is implied when usingthis.listenTo()
. -
-
reviewboard/static/rb/js/models/reviewRequestEditorModel.es6.js (Diff revisions 2 - 3) Sentence casing.