converted remaining text-track modules to full es6 #3130
Conversation
added jsdoc comments to functions that did not previously have them linted changed files with vjsstandard
| }; | ||
|
|
||
| /* jshint ignore:start */ |
There was a problem hiding this comment.
it probably doesn't understand const.
|
Made some comments, mostly minor changes. |
updated several jsdoc comments removed the indexOf polyfill as we assume es5 and use the standard one
|
@gkatsev OK I think I addressed your comments |
| @@ -12,72 +13,93 @@ import document from 'global/document'; | |||
| * getter TextTrackCue (unsigned long index); | |||
| * TextTrackCue? getCueById(DOMString id); | |||
| * }; | |||
| * | |||
| * @param {Array} cues cues to add to the list | |||
There was a problem hiding this comment.
This comment also needs to be updated. cues to be initialized with or something.
gkatsev
added
patch
| get() { | ||
| return kind; | ||
| }, | ||
| set: () => {} |
There was a problem hiding this comment.
This is in the nitpicky category but I'm wondering if we should encode noops like this:
{
set(): {}
}The fat arrow functions have extra functionality we don't want here and I think they'll end up getting transpiled into more code.
There was a problem hiding this comment.
Looks like it's smart enough to not try anything: http://babeljs.io/repl/#?experimental=true&evaluate=true&loose=true&spec=false&playground=true&code=let%20o%20%3D%20{%0A%20%20set%28%29%20{}%2C%0A%20%20set2%3A%20%28%29%20%3D%3E%20{}%0A}%3B%0A%0Alet%20o2%20%3D%20{...o}%3B
However, I do like just using the shorthand, makes it cleaner.
There was a problem hiding this comment.
I'm in favor of sticking with the shorthand syntax for consistency (but good to know it gets optimized in the transpiling).
|
Other than the comment about fat-arrow functions, lgtm |
|
LGTM |
gkatsev
added
confirmed
and removed
needs: LGTM
Description
Converted several text-track files to full es6 and added jsdoc comments to the files that I touched
Specific Changes proposed
Requirements Checklist