Don't use ES6 syntax in gulp task definitions#9107
Don't use ES6 syntax in gulp task definitions#9107jridgewell merged 3 commits intoampproject:masterfrom rsimha:2017-05-02-FixVisualDiff
Conversation
jridgewell
left a comment
There was a problem hiding this comment.
- All the
consts String#includeson L58
build-system/tasks/visual-diff.js
Outdated
| var p = | ||
| child_process.spawnSync('/bin/sh', ['-c', cmd], {'stdio': 'inherit'}); | ||
| if (p.status != 0) { | ||
| console/*OK*/.log( |
| var percyArgs = extractPercyArgs(); | ||
| commandLine.push('--baseurl /' + percyArgs.webpage); | ||
| commandLine.push('--widths ' + percyArgs.widths); | ||
|
|
|
I tested my original PR by running "gulp test" before submitting it. Could this be a difference between running gulp test on macOS vs. Linux? |
|
What version of Node are you running? ( |
|
I'm using node v7.9.0. |
|
Ah, that version supports most of ES6. I'm using v4.8.3, which, according to package.json, is the version that AMP officially supports. Should we consider just bumping the required Node version and then use ES6 freely in Node scripts? |
|
Also, I observe that Travis is configured to run against the latest stable release. |
|
Yes. I noticed that there is ES6 syntax in other js files in build-system. There certainly is ES6 code in pr-check.js, which runs against every PR. I'll keep this in mind for later. Meanwhile, I still think there's value in checking in the changes in this PR, since others on the team are likely to run into what you just saw. I've pushed a new commit with (hopefully) all the changes you requested. |
|
@jridgewell or @taymonbeal, could one of you merge this PR? (I don't have permissions yet.) |
This PR fixes all instances of ES6 syntax in visual-diff.js.
FIxes #9106