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

Move lint configuration to react-scripts fork #54

Merged
merged 2 commits into from
May 11, 2017

Conversation

randycoulman
Copy link
Contributor

@randycoulman randycoulman commented May 10, 2017

We were duplicating the list of lint plugins between this project and our react-scripts fork. Now, all lint-related packages are installed via react-scripts, and we run the linter manually using its the new react-scripts lint command.

This also gets us up-to-date with the latest eslint-config-zeal and friends.

The one potential issue with this change is the ability to get lint feedback in a text editor, but that seems to be working in our testing. If we run into issues on other systems, we’ll have to revisit.

As part of our testing, we discovered that the generated project did not have the test-specific .eslintrc.json file, so was getting lint errors right after generation. We updated the copy command in the generator to include dotfiles.

We've confirmed that, in a generated project, we get the same output from:

  • yarn lint:js
  • yarn start
  • In the browser console
  • In Atom

Closes #45
Fixes #48
Fixes #50

Randy Coulman added 2 commits May 10, 2017 10:08
The generated project did not have the test-specific .eslintrc.json
file, so was getting lint errors right after generation.
@randycoulman randycoulman requested review from lexun and seanculver May 10, 2017 17:23
@randycoulman randycoulman merged commit 6013b48 into master May 11, 2017
@randycoulman randycoulman deleted the features/move-lint-config-to-react-scripts branch May 11, 2017 15:47
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