chore(*): switch from JSHint/JSCS to ESLint (+ other build-related changes) #14952

mgol · 2016-07-26T09:42:43Z

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
A build-related refactor.

What is the current behavior? (You can also link to an open issue here)
Code is linted via JSHint+JSCS.

What is the new behavior (if this is a feature change)?
Code is linted via ESLint.

Does this PR introduce a breaking change?
No

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/angular/angular.js/blob/master/CONTRIBUTING.md#commit-message-format
~~Tests for the changes have been added (for bug fixes / features)~~
~~Docs have been added / updated (for bug fixes / features)~~

Other information:
This is not ready (there are still many ESLint violations) but since the changes are rather large, I'd like to get some feedback now.

There are a couple of build-related commits; submitting them as separate PRs would be difficult as they modify package.json and shrinkwrap file. It makes sense to land them separately and all but the last commit are small so it shouldn't case lots of problems in reviewing.

gkalpak · 2016-07-26T11:30:29Z

.eslintrc.json

+    "accessor-pairs": "error",
+    "array-callback-return": "error",
+    "complexity": ["warn", 10],
+//    "dot-notation": "error", // TODO enable?


I think there are places where we deliberately use bracket-notation. If there are many such places (that need a "manual" override), I would leave this disabled.

I was thinking about moving all those TODO rules to a separate
.eslintrc-todo.json file, then discuss all the options, remove the one we
don't want to enable and the rest will stay there, waiting for someone to
fix the code and enable the rule.

Thoughts?

On Tue, Jul 26, 2016 at 1:32 PM, Georgios Kalpakas <[email protected]

wrote:

In .eslintrc.json
#14952 (comment):

"no-irregular-whitespace": "error",

"no-negated-in-lhs": "error",

"no-obj-calls": "error",

"no-regex-spaces": "error",

"no-sparse-arrays": "error",

"no-unreachable": "error",

"use-isnan": "error",

"no-unsafe-finally": "error",

"valid-typeof": "error",

"no-unexpected-multiline": "error",

// Best practices

"accessor-pairs": "error",

"array-callback-return": "error",

"complexity": ["warn", 10],
+// "dot-notation": "error", // TODO enable?

I think there are places where we deliberately use bracket-notation. If
there are many such places (that need a "manual" override), I would leave
this disabled.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/angular/angular.js/pull/14952/files/6f56b890138f16a67b5f61cb2238d75378a76497..22f65cbbbf36f1ae396038f457ca2266f5b52767#r72231300,
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABrUnvg_rt1IyxEc7jYUKkY9aFjn3a20ks5qZfBkgaJpZM4JU90D
.

Michał Gołębiowski

gkalpak · 2016-07-26T12:55:59Z

test/ng/compileSpec.js

-    it('should not sanitize attributes other than src', inject(function($compile, $rootScope) {
-      /* jshint scripturl:true */
-      element = $compile('<img title="{{testUrl}}"></img>')($rootScope);
+    it('should not sanitize attributes other than src', inject(function($compile, $rootScope) {      element = $compile('<img title="{{testUrl}}"></img>')($rootScope);


Missing newline. (Below too.)

gkalpak · 2016-07-26T13:33:25Z

So far, this LGTM 👍

Except where otherwise noted, I like (eventually) enabling the rules marked as TODO enable?, but I think it is better to leave them disabled at first and discuss/enable them in follow-up PRs.

googlebot · 2016-07-27T21:14:13Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-07-27T21:14:13Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

gkalpak · 2016-08-05T16:00:48Z

Still LGTM2 😄

… packages The Travis build.sh script has to be updated as the previous way of specifying parameters to the code run in Grunt tasks stopped working with the newest Grunt.

The previous version depended on a vulnerable request version. Ref angulargh-14961

This avoids warnings with newer versions of Bower.

If one uses nvm to manage Node.js versions, the .nvmrc file makes `nvm use` switch to the version specified in .nvmrc. There are scripts that invoke it automatically when cd'ing to directories containing .nvmrc so that you never run build commands using a wrong Node version, see: https://github.com/creationix/nvm/blob/v0.31.2/README.markdown#zsh

The engineStrict field is deprecated in npm 2 and removed in npm 3.

The "licenses" field is deprecated in favor of the "license" field... which we already have specified.

@Narretz

Thanks to @Narretz for help in fixing style violations and to @gkalpak for a very extensive review.

Partially cherry-picked from c322032 Ref angular#14952

mgol · 2016-08-05T21:51:54Z

All commits except the ESLint one backported to v1.5.x. The last one is handled in PR #14993; please review.

Partially cherry-picked from c322032 Ref angular#14952

@this

Related to angular#14952. Fixed the following warnings/errors: 1. **Warning**: Closure Compiler complained about `/* @this */` (annotations in non-JSDoc cooments). Fixed by changing `/* @this */` to `/** @this */`. 2. **Warning**: Dgeni complaines about `/** @this */` (invalid tags found). Fixed by adding an empty `this` tag definition in `docs/config/tag-defs/`. 3. **Error**: ESLint complained about CRLF linebreaks in `build/docs/examples/`. These are generated by dgeni and (apparently) use the system's default linebreak (i.e. CRLF on Windows). Fixed by disabling the `linebreak-style` rule for `build/docs/examples/`.

Partially cherry-picked from c322032 Ref #14952 Closes #14993

This is a followup to the migration to ESLint. Ref angular#14952

This is a followup to the migration to ESLint. Ref #14952 Closes #15006

@this

Related to #14952. Fixed the following warnings/errors: 1. **Warning**: Closure Compiler complained about `/* @this */` (annotations in non-JSDoc comments). Fixed by changing `/* @this */` to `/** @this */`. 2. **Warning**: Dgeni complained about `/** @this */` (invalid tags found). Fixed by adding an empty `this` tag definition in `docs/config/tag-defs/`. 3. **Error**: ESLint complained about CRLF linebreaks in `build/docs/examples/`. These are generated by dgeni and (apparently) use the system's default linebreak (e.g. CRLF on Windows). Fixed by disabling the `linebreak-style` rule for `build/docs/examples/`. Closes #14997

@this

Related to #14952. Fixed the following warnings/errors: 1. **Warning**: Closure Compiler complained about `/* @this */` (annotations in non-JSDoc comments). Fixed by changing `/* @this */` to `/** @this */`. 2. **Warning**: Dgeni complained about `/** @this */` (invalid tags found). Fixed by adding an empty `this` tag definition in `docs/config/tag-defs/`. 3. **Error**: ESLint complained about CRLF linebreaks in `build/docs/examples/`. These are generated by dgeni and (apparently) use the system's default linebreak (e.g. CRLF on Windows). Fixed by disabling the `linebreak-style` rule for `build/docs/examples/`. Closes #14997

googlebot added the cla: yes label Jul 26, 2016

gkalpak reviewed Jul 26, 2016
View reviewed changes

mgol force-pushed the eslint-etc branch from 22f65cb to 9bb014b Compare July 26, 2016 12:54

gkalpak reviewed Jul 26, 2016
View reviewed changes

mgol force-pushed the eslint-etc branch from 9bb014b to f807c2f Compare July 26, 2016 14:26

gkalpak added this to the 1.5.x milestone Jul 27, 2016

gkalpak added the type: chore label Jul 27, 2016

mgol force-pushed the eslint-etc branch from f807c2f to 3141607 Compare July 27, 2016 12:21

googlebot added cla: no and removed cla: yes labels Jul 27, 2016

mgol force-pushed the eslint-etc branch from 72c4337 to e2303e5 Compare July 28, 2016 22:56

mgol added cla: yes and removed cla: no labels Jul 28, 2016

mgol force-pushed the eslint-etc branch from e2303e5 to 5e706ab Compare July 29, 2016 09:54

mgol force-pushed the eslint-etc branch from 9d6f247 to 295b42e Compare August 5, 2016 17:30

mgol added 8 commits August 5, 2016 19:37

chore(package.json): add grunt-cli to devDependencies, update grunt-*…

3134e78

… packages The Travis build.sh script has to be updated as the previous way of specifying parameters to the code run in Grunt tasks stopped working with the newest Grunt.

chore(package.json): upgrade Protractor

0606b15

The previous version depended on a vulnerable request version. Ref angulargh-14961

chore(bower): change the name in bower.json

bdb794e

This avoids warnings with newer versions of Bower.

chore(package.json): remove the engineStrict field

6f73729

The engineStrict field is deprecated in npm 2 and removed in npm 3.

chore(package.json): update Karma & BrowserStack-related packages

8c00386

chore(package.json): Remove the deprecated licenses field

517b9bd

The "licenses" field is deprecated in favor of the "license" field... which we already have specified.

chore(*): switch from JSHint/JSCS to ESLint

30c027e

Thanks to @Narretz for help in fixing style violations and to @gkalpak for a very extensive review.

mgol force-pushed the eslint-etc branch from 295b42e to 30c027e Compare August 5, 2016 19:44

mgol closed this in c322032 Aug 5, 2016

mgol deleted the eslint-etc branch August 5, 2016 20:19

mgol added a commit to mgol/angular.js that referenced this pull request Aug 5, 2016

chore(*): switch from JSHint/JSCS to ESLint

19d452c

Partially cherry-picked from c322032 Ref angular#14952

mgol mentioned this pull request Aug 5, 2016

chore(*): switch from JSHint/JSCS to ESLint #14993

Closed

3 tasks

mgol added a commit to mgol/angular.js that referenced this pull request Aug 5, 2016

chore(*): switch from JSHint/JSCS to ESLint

bbead8c

Partially cherry-picked from c322032 Ref angular#14952

mgol added a commit to mgol/angular.js that referenced this pull request Aug 5, 2016

chore(*): switch from JSHint/JSCS to ESLint

58829ce

Partially cherry-picked from c322032 Ref angular#14952

mgol added a commit to mgol/angular.js that referenced this pull request Aug 5, 2016

chore(*): switch from JSHint/JSCS to ESLint

1824cc2

Partially cherry-picked from c322032 Ref angular#14952

gkalpak mentioned this pull request Aug 6, 2016

chore(package): fix some warnings/errors #14997

Closed

3 tasks

gkalpak pushed a commit that referenced this pull request Aug 8, 2016

chore(*): switch from JSHint/JSCS to ESLint

b1bc251

Partially cherry-picked from c322032 Ref #14952 Closes #14993

mgol added a commit to mgol/angular.js that referenced this pull request Aug 8, 2016

chore(*): minor code style tweaks

6afc686

This is a followup to the migration to ESLint. Ref angular#14952

mgol added a commit to mgol/angular.js that referenced this pull request Aug 8, 2016

chore(*): minor code style tweaks

0dcf4ac

This is a followup to the migration to ESLint. Ref angular#14952

mgol mentioned this pull request Aug 8, 2016

chore(*): minor code style tweaks #15006

Closed

3 tasks

gkalpak pushed a commit that referenced this pull request Aug 8, 2016

chore(*): minor code style tweaks

49f0777

This is a followup to the migration to ESLint. Ref #14952 Closes #15006

gkalpak pushed a commit that referenced this pull request Aug 8, 2016

chore(*): minor code style tweaks

4d753d1

This is a followup to the migration to ESLint. Ref #14952 Closes #15006

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(*): switch from JSHint/JSCS to ESLint (+ other build-related changes) #14952

chore(*): switch from JSHint/JSCS to ESLint (+ other build-related changes) #14952

mgol commented Jul 26, 2016

gkalpak Jul 26, 2016

mgol Jul 26, 2016

gkalpak Jul 26, 2016

gkalpak Jul 26, 2016 •

edited

Loading

gkalpak commented Jul 26, 2016

googlebot commented Jul 27, 2016

googlebot commented Jul 27, 2016

gkalpak commented Aug 5, 2016

mgol commented Aug 5, 2016

chore(*): switch from JSHint/JSCS to ESLint (+ other build-related changes) #14952

chore(*): switch from JSHint/JSCS to ESLint (+ other build-related changes) #14952

Conversation

mgol commented Jul 26, 2016

gkalpak Jul 26, 2016

Choose a reason for hiding this comment

mgol Jul 26, 2016

Choose a reason for hiding this comment

gkalpak Jul 26, 2016

Choose a reason for hiding this comment

gkalpak Jul 26, 2016 • edited Loading

Choose a reason for hiding this comment

gkalpak commented Jul 26, 2016

googlebot commented Jul 27, 2016

googlebot commented Jul 27, 2016

gkalpak commented Aug 5, 2016

mgol commented Aug 5, 2016

gkalpak Jul 26, 2016 •

edited

Loading