-
Notifications
You must be signed in to change notification settings - Fork 18
Update Project to have ES6 Conventions #84
base: develop
Are you sure you want to change the base?
Conversation
commands/preview.js
Outdated
|
||
exports.command = function(url, bundle, callback) { | ||
var browser = this; | ||
exports.command = function(url, bundle = 'https://localhost:8443/loader.js', callback) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that you're using es6 everything, do you still need to create the exports object? Or can you just say:
export const command = function(...)
Also, while you're at it, can you use an arrow function here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed. Unfortunately we're unable to use an arrow function due to losing the ability to call 'argument.lengths'.
commands/triggerClick.js
Outdated
@@ -1,13 +1,13 @@ | |||
exports.command = function(selector, callback) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment about exports here
"test": "./node_modules/.bin/grunt lint; ./node_modules/.bin/grunt test" | ||
"test": "./node_modules/.bin/grunt lint; ./node_modules/.bin/grunt test", | ||
"lint": "npm run lint:js", | ||
"lint:js": "eslint '**/*.{js,jsx}'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to double check that this blob works on Windows. I've run into issues with commands before where the quotes used for this mattered to Windows and broke things. 😞
@@ -20,7 +24,9 @@ | |||
}, | |||
"scripts": { | |||
"install": "node selenium/install.js", | |||
"test": "./node_modules/.bin/grunt lint; ./node_modules/.bin/grunt test" | |||
"test": "./node_modules/.bin/grunt lint; ./node_modules/.bin/grunt test", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be ;
or &&
? I don't actually remember what ;
does 😅
Status: Ready for Review or Open for Visibility
Owner:
Reviewers: @ellenmobify @jasecode
Changes
Todos:
grunt lint
)Feedback:
none so far
How To Test
npm install
npm link
npm run lint