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

chore: use standard linter locally #62

Merged
merged 4 commits into from
Aug 3, 2019

Conversation

erickzhao
Copy link
Contributor

@erickzhao erickzhao commented Jul 27, 2019

Hi there,

I saw that the build on this project was failing due to code style issues, but there was no way to check the linting status beyond running on the CI or having standard installed globally.

I installed ESLint as a dev dependency (with the StandardJS preset) to make it easier to lint locally. npm run lint now detects errors locally. Since that command is now available, I added the local linter to the CI.

I also fixed a bug with TravisCI where the config ran on Node 6, which is no longer supported by core nor StandardJS. I replaced it with the latest LTS version of Node.

@yan-foto
Copy link
Owner

Thanks for the pull request.

"ESLint or Standard" is one of those ongoing struggles (like "vim vs nano vs emacs") that seems to remain unresolved for all eternity! That being said, I see no advantages in using eslint over standard. If the only problem is "having to install standard globally", you could solve it the same way that you did with eslint, namely by putting standard in the devDependencies and executing it as a node scricpt in package.json, i.e.:

"scripts": {
  "lint": "standard main.js"
}

and run node run lint.

I like the other changes that you made, so I'll keep this PR open, but I'd replace eslint with standard both in devDependencies and scripts. What do you reckon?

@erickzhao
Copy link
Contributor Author

@yan-foto Yeah, using ESLint with the StandardJS preset instead of the standard package directly doesn't have direct benefits on the code, since it ends up being the same linting rules. I could definitely change this PR to use standard directly instead.

@yan-foto
Copy link
Owner

yan-foto commented Jul 28, 2019 via email

@erickzhao erickzhao changed the title chore: use ESLint chore: use standard linter locally Jul 28, 2019
@erickzhao
Copy link
Contributor Author

@yan-foto done! :)

@yan-foto
Copy link
Owner

Great! Thanks again. I'll merge this into develop first, increase version number and merge with master.

@yan-foto yan-foto merged commit 5058397 into yan-foto:master Aug 3, 2019
@erickzhao erickzhao deleted the fix/local-linting branch August 4, 2019 00:40
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