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

feat: support .eslintrc #36

Closed
wants to merge 3 commits into from
Closed

feat: support .eslintrc #36

wants to merge 3 commits into from

Conversation

ericclemmons
Copy link
Member

Similar to #23.

@ericclemmons ericclemmons self-assigned this Apr 15, 2016
@ericclemmons ericclemmons added this to the 3.1.0 milestone Apr 15, 2016
@ericclemmons
Copy link
Member Author

@ericclemmons
Copy link
Member Author

Hmmm, maybe not. This looks like it's not easily discernible what all of the dependencies are, because of extends:

https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb

@ericclemmons ericclemmons removed this from the 3.1.0 milestone Apr 25, 2016
@ericclemmons
Copy link
Member Author

It may be possible since peerDependencies are auto-installed now...

@ericclemmons ericclemmons reopened this Apr 25, 2016
@ericclemmons ericclemmons modified the milestone: 4.1.0 May 23, 2016
@ericclemmons
Copy link
Member Author

I don't know how I feel about this. It's nice that it's automated, but only know of a handful of examples using extends.

@insin @poeticninja @bebraw What do you think? It certainly lowers the barrier for starting a project (e.g. Add .babelrc, .eslintrc, and start coding), but would want to stop supporting "external tools" at just Babel & ESLint.

@ericclemmons
Copy link
Member Author

ericclemmons commented May 26, 2016

Examples to support:

  • Extends eslint:recommended
  • Extends airbnb
  • Extends [ "airbnb", "eslint:recommended" ] (e.g. multiple configs)
  • Extends @highereducation/eslint-config

@bebraw
Copy link

bebraw commented May 26, 2016

@ericclemmons It's a niche case for sure. Probably supporting Babel and ESLint for now gets you far. Accept the rest as PRs. 👍

@poeticninja
Copy link
Contributor

Pretty much everyone that is using ESLint (that I have seen) are extending from some other place because there are a lot of rules. Very very few people are actually writing their own rules (I did once and not again). I think supporting extends is important to support ESLint.

@ericclemmons
Copy link
Member Author

Cool, I think I got this functional. If you guys wanna give it a shot, go for it!

@coveralls
Copy link

coveralls commented May 31, 2016

Coverage Status

Coverage increased (+0.5%) to 93.956% when pulling b74202c on 36-eslint into 7649f30 on master.

@poeticninja
Copy link
Contributor

I just tested it out with example/webpack1. I ran npm start... it installed all of the dependencies. After that I created an .eslintrc file with configurations extending airbnb as a test. I had to turn off and run npm start again for the install to take place again. BUT it worked!! 👍

@ericclemmons
Copy link
Member Author

Here's a good test for ya:

https://www.npmjs.com/package/eslint-config-airbnb

This thing has so many peerDependencies and requirements it's absurd.

@ericclemmons
Copy link
Member Author

@insin What do you think about this feature? Otherwise I'm considering closing it...

try {
var options = JSON.parse(
fs.readFileSync(
path.join(process.cwd(), ".eslintrc"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are more possible formats with extensions for e.g .eslintrc.yml, .eslintrc.js and so forth

@michael-ciniawsky
Copy link
Member

Would this be really needed ?

@alexander-akait
Copy link
Member

@michael-ciniawsky imho, seems no

@michael-ciniawsky
Copy link
Member

👍 || 👎 on my comment above please :)

@michael-ciniawsky
Copy link
Member

Closing due to inactivity for now, but reopen if still relevant :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants