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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Eslint #9197

Merged
merged 11 commits into from Nov 1, 2017
Merged

Eslint #9197

merged 11 commits into from Nov 1, 2017

Conversation

ErisDS
Copy link
Member

@ErisDS ErisDS commented Nov 1, 2017

Had a go at switching to eslint.

Somewhere in 417d690 I think I broke something 馃槺 can anyone spot what?

What I did was:

  • run eslint --init and choose the "inspect code" option.
  • I did this for all server files, and then all test files separately
  • then I fixed the handful of issues
  • then I changed a few style rules that we have always had, only eslint is way better at this stuff
  • I went through loads of files using the Ctrl + Alt + L shortcut to fix indent and other problems
  • finally, I disabled a lot of style rules for the tests cos seriously this isn't important

@@ -21,6 +21,7 @@ utils = {
/**
* Timespans in seconds and milliseconds for better readability
*/
/* eslint-disable key-spacing
ONE_HOUR_S: 3600,

This comment was marked as abuse.

This comment was marked as abuse.

@ErisDS
Copy link
Member Author

ErisDS commented Nov 1, 2017

OK so this is green now. I propose:

  • Merge now as-is.
  • If there are any rules that really annoy you, turn them off when you find them.
  • If there is anything that is not being picked up, add to the rules when you find them.

There are some rules that I already turned off that we might want to look at again:

  • no-useless-escape
  • vars-on-top
  • max-len
  • no-buffer-constructor
  • array-callback-return

It takes quite a lot to really research a rule, apply it, update all the code etc. Doing one rule at a time will be a lot easier than trying to do all of them in one go was 馃榿

P.S. ESLint is ~40seconds faster, reducing lint time on travis from ~2.20 to ~1:40

@ErisDS ErisDS merged commit bcf5a1b into TryGhost:master Nov 1, 2017
@ErisDS ErisDS deleted the eslint branch November 1, 2017 13:44
@ErisDS ErisDS removed their assignment Jun 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants