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

Enable a minimal ESLint configuration and fix violations #178

Closed
wants to merge 1 commit into from

Conversation

astorije
Copy link
Contributor

More comprehensive version of #154

Configuration was created with npm run lint -- --init (plus minor tweaks like re-allowing console.log), and fixing violation was done with npm run lint -- --fix followed by very few minual fixes.
This will also be enforced by CI to make sure future PRs respect this.

This is how the config file was created:

screen shot 2017-11-25 at 22 30 55

The .eslintrc.yml file created is very minimal, I recommend going through https://eslint.org/docs/rules/ to see what is important to you after this PR.

⚠️ Note that some removals were the consequences of ESLint informing of a chain of unused variables. I tried my best but did not test this carefully beyond npm test so I might have removed test code that was supposed to stay.

Configuration was created with `npm run lint -- --init` (plus minor tweaks like re-allowing `console.log`), and fixing violation was done with `npm run lint -- --fix` followed by very few minual fixes.
This will also be enforced by CI.
putermancer added a commit that referenced this pull request Dec 29, 2017
@putermancer
Copy link
Collaborator

Due to another PR that I merged first, this one ended up having a minor merge conflict in tap_reporter.js. I checked out this branch locally, rebased your changes to resolve the TAP reporter conflict, and restored some of the original behavior that this PR had removed (probably because it was using a variable that was never meaningfully used when this PR was originally created).

Thanks for the PR, sorry for my slowness :)

@astorije astorije deleted the astorije/eslint branch December 29, 2017 06:30
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.

2 participants