Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace jshint with eslint #942

Closed
flekschas opened this issue Mar 12, 2016 · 5 comments · Fixed by #1023
Closed

Replace jshint with eslint #942

flekschas opened this issue Mar 12, 2016 · 5 comments · Fixed by #1023
Assignees
Milestone

Comments

@flekschas
Copy link
Member

ESLint is heavily developed and supported by one of the smartes guys in the JS scene. It's pluggable (offering loooots of good rules for formatting code) and is used by major style guides like Airbnb.

@flekschas flekschas self-assigned this Mar 12, 2016
@flekschas flekschas added this to the Quincy milestone Mar 12, 2016
@flekschas
Copy link
Member Author

I've exchanged jshint with eslint. You can check it out in branch flekschas/eslint. I've also installed Airbnb's style guide and fixed all issues in Gruntfile.js but we might want to hold of with merging it into develop because we have a lot of issues to fix:

$ grunt eslint
Running "eslint:sourceCode" (eslint) task
...
✖ 9675 problems (9574 errors, 101 warnings)

Warning: Task "eslint:sourceCode" failed. Use --force to continue.

Aborted due to warnings.

This issues are no show stoppers but if we want to have really clean, good-looking code we should address them at some point.

Example list of errors:
(Can't copy all issues because the message gets too long...)

$ grunt eslint
Running "eslint:sourceCode" (eslint) task

/Users/Fritz/Sites/refinery/refinery/ui/source/js/workflows.js
   1:1   error  Use the global form of 'use strict'                                           strict
   3:44  error  Missing space before function parentheses                                     space-before-function-paren
   4:3   error  Use the global form of 'use strict'                                           strict
   6:36  error  Missing space before function parentheses                                     space-before-function-paren
   7:45  error  Missing space before function parentheses                                     space-before-function-paren
  16:42  error  Missing space before function parentheses                                     space-before-function-paren
  20:24  error  Strings must use singlequote                                                  quotes
  24:1   error  Block must not be padded by blank lines                                       padded-blocks
  26:10  error  Strings must use singlequote                                                  quotes
  26:33  error  Missing space before function parentheses                                     space-before-function-paren
  27:3   error  Use the global form of 'use strict'                                           strict
  30:5   error  Strings must use singlequote                                                  quotes
  30:26  error  A space is required after '{'                                                 object-curly-spacing
  30:35  error  Strings must use singlequote                                                  quotes
  30:41  error  A space is required before '}'                                                object-curly-spacing
  34:10  error  Strings must use singlequote                                                  quotes
  34:30  error  Missing space before function parentheses                                     space-before-function-paren
  34:31  error  '$log' is defined but never used                                              no-unused-vars
  35:3   error  Use the global form of 'use strict'                                           strict
  39:30  error  Missing space before function parentheses                                     space-before-function-paren
  40:28  error  Unnecessary use of boolean literals in conditional expression                 no-unneeded-ternary
  43:22  error  Missing space before function parentheses                                     space-before-function-paren
  47:22  error  Missing space before function parentheses                                     space-before-function-paren
  51:32  error  Missing space before function parentheses                                     space-before-function-paren
  56:12  error  Unexpected 'else' after 'return'                                              no-else-return
  56:12  error  Closing curly brace does not appear on the same line as the subsequent block  brace-style
  60:10  error  Closing curly brace does not appear on the same line as the subsequent block  brace-style
  60:10  error  Unexpected 'else' after 'return'                                              no-else-return
  63:7   error  Expected indentation of 4 space characters but found 6                        indent
  66:18  error  Expected to return a value at the end of this function                        consistent-return
  66:26  error  Missing space before function parentheses                                     space-before-function-paren
  70:21  error  Expected to return a value at the end of this function                        consistent-return
  70:29  error  Missing space before function parentheses                                     space-before-function-paren
  74:18  error  Expected to return a value at the end of this function                        consistent-return
  74:26  error  Missing space before function parentheses                                     space-before-function-paren
  78:30  error  Missing space before function parentheses                                     space-before-function-paren
  80:14  error  Strings must use singlequote                                                  quotes
  82:10  error  Unexpected 'else' after 'return'                                              no-else-return
  82:10  error  Closing curly brace does not appear on the same line as the subsequent block  brace-style
  83:62  error  Strings must use singlequote                                                  quotes
  87:22  error  Expected to return a value at the end of this function                        consistent-return
  87:30  error  Missing space before function parentheses                                     space-before-function-paren
  89:7   error  Expected space(s) after "switch"                                              keyword-spacing
  89:7   error  Expected a default case                                                       default-case
  96:30  error  Expected to return a value at the end of this function                        consistent-return
  96:38  error  Missing space before function parentheses                                     space-before-function-paren

✖ 8233 problems (8132 errors, 101 warnings)

@ngehlenborg ngehlenborg removed this from the Quincy milestone Mar 22, 2016
@flekschas
Copy link
Member Author

This might be a very useful preset as well. It comes with tons of Angular best practices such as using controllerAs:
https://github.com/Gillespie59/eslint-plugin-angular

@flekschas
Copy link
Member Author

After setting up esformatter and going through commons, dashboard, globals, node-mapping, and node-relationships we're down to: 6657 problems (6604 errors, 53 warnings). (Commit: 83e837b)

tl/dr:
This means that the listed features are 100% linting error free.

@flekschas
Copy link
Member Author

After applying the auto-formatting to all JS files we are down to 1846 problems (1805 errors, 41 warnings). (Commit: dc136e3)

It is not the most exiting thing to do but it's definitely possible to fix all those issues in less then one day. (I went through ~2000 today with configuring esformatter.)

Lesson learn: Apart from many style related fixes, I was able to fix a couple of more serious bugs and removed lots of unused / old code.

@flekschas flekschas added this to the Next milestone Apr 11, 2016
@flekschas
Copy link
Member Author

Formatted and cleaned Antons code.

Without the provvis* we're down to: 697 problems (668 errors, 29 warnings)

*) The provvis directory needs a more thorough refactoring, see here #1000.

In order to be able to merge the code we could also ignore other directories as well.

@ngehlenborg ngehlenborg modified the milestones: Next, Salem Apr 12, 2016
mccalluc added a commit that referenced this issue Aug 1, 2017
mccalluc added a commit that referenced this issue Aug 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants