Partially fix existing lint errors in build-system/
#11334
Conversation
|
/to @erwinmombay @choumx This PR reduces the number of lint errors in |
rsimha
changed the title
build-system/build-system/
build-system/tasks/changelog.js
Outdated
| @@ -437,11 +437,11 @@ function getClosedPullRequests(opt_page) { | |||
| options.qs = { | |||
| state: 'closed', | |||
| page: opt_page, | |||
| access_token: GITHUB_ACCESS_TOKEN, | |||
There was a problem hiding this comment.
Could we change the policy to align with our non-build code?
There was a problem hiding this comment.
We currently use the rules in eslint-config-es5 because the build system code is written in ES5 JS, while the runtime is in ES6. That's where the non-conforming comma rules are coming from.
I'm working on a parallel thread on upgrading to using node 6 on Travis, at which point we can safely use ES6 code in build-system, and then use the same linter as the rest of the runtime. Will update this PR once I have that part figured out.
There was a problem hiding this comment.
We now use the same ES6 rules as the rest of the AMP runtime, and trailing commas are back. PTAL.
|
You should be able to opt into trailing comma. Totally valid ES5.
…On Tue, Sep 19, 2017 at 1:13 PM, Raghu Simha ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In build-system/tasks/changelog.js
<#11334 (comment)>:
> @@ -437,11 +437,11 @@ function getClosedPullRequests(opt_page) {
options.qs = {
state: 'closed',
page: opt_page,
- access_token: GITHUB_ACCESS_TOKEN,
We currently use the rules in eslint-config-es5 because the build system
code is written in ES5 JS, while the runtime is in ES 6. That's where the
non-conforming comma rules are coming from.
I'm working on a parallel thread on upgrading to using node 6 on Travis,
at which point we can safely use ES6 code in build-system, and then use
the same linter as the rest of the runtime. Will update this PR once I have
that part figured out.
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#11334 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAFeT8NXvsiRO9jkW2QYIelLN6C2Saygks5skCBRgaJpZM4PcyBT>
.
|
|
Right, what I meant to say was that with the impending move to node 6 on Travis, we no longer need to restrict build system code to ES5, and therefore, no longer need separate sets of lint rules. For context on why we're doing so, see the discussion starting at #11278 (comment) |
|
The latest commit in this PR uses ES6 rules, and after an automatic @cramforce PTAL. |
.eslintrc
Outdated
| @@ -33,7 +33,10 @@ | |||
| "sandbox": true, | |||
| "context": false, | |||
| "global": false, | |||
| "describes": true | |||
| "describes": true, | |||
| "require": false, | |||
There was a problem hiding this comment.
Please move these into a sub folder's .eslintrc. They're dangerous in the src folder.
There was a problem hiding this comment.
Good point. I was about to ask. Done.
rsimha
dismissed
cramforce’s stale review
We now use the same lint rules across all directories. Trailing commas are back.
This PR contains all possible automatic lint fixes for
build-systemby running:There are still a bunch of errors that must be dealt with via manual fixes, or eslint rule changes.
Follow up to #11325
Partial fix for #11273