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

ESLint everywhere #40

Merged
merged 38 commits into from
Feb 8, 2017
Merged

ESLint everywhere #40

merged 38 commits into from
Feb 8, 2017

Conversation

indexzero
Copy link
Contributor

@indexzero indexzero commented Feb 1, 2017

Fixes all open issues: #16, #17, #27, #32

This PR removes jscs and replaces all rules with eslint auto-fix. See jscs to eslint research. Since this now uses a single platform, fashion-show is no longer necessary.

The new API surface area is three eslint-config-** packages:

  • eslint-config-godaddy: Base configuration for non-React, ES6 JavaScript applications
  • eslint-config-godaddy-react: Configuration for ES6 React JavaScript applications
  • eslint-config-godaddy-es5: Configuration for React and non-React ES5 JavaScript applications.

package.json Outdated
@@ -27,24 +28,19 @@
"peerDependencies": {
"eslint-plugin-mocha": "^2.0.0",
"eslint-plugin-react": "^4.1.0",
"eslint-plugin-json": "^1.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

These files should become dependencies instead of peer or devDependencies as our configuration extend these files. See #31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was following the pattern established in airbnb/javascript for peerDependencies.

... puts on thinking cap ...

If we want to be backwards compatible for both npm@2 AND npm@3 then I think we need to keep these in peerDependencies (but obviously make them more liberal – like "*"). If we want to only support npm@3 then we can keep them as dependencies since they will be flattened by default.

Another option (which I have NO idea will work) is to use the require.resolve pattern used for "extends" for "plugins". i.e. change:

{
  plugins: ['mocha', 'json'],
}

to use require.resolve:

{
  plugins: ['eslint-plugin-mocha', 'eslint-plugin-json']
    .map(require.resolve),
}

Copy link
Member

Choose a reason for hiding this comment

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

As a side note I am definitely all for maintaining backwards compatibility. Option 2 is definitely an unknown.

@samshull
Copy link
Contributor

samshull commented Feb 2, 2017

👏 Thanks for the work @indexzero! Looks great!

@rxmarbles
Copy link
Member

👍 great work!

@chemdrew
Copy link

chemdrew commented Feb 2, 2017

👍

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.

5 participants