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

Add recommended config #71

Merged
merged 1 commit into from
Oct 24, 2019
Merged

Conversation

brettz9
Copy link
Contributor

@brettz9 brettz9 commented Sep 17, 2019

  • Enhancement: Add "recommended" config which auto-adds plugins and all current rules (one as a warning); fixes Add recommended config #69
    - Linting: Rename eslintrc to include recommended extension
    - git/npm: Add package-lock.json to ignored files
    - npm: Add recommended package.json fields (contributors, dependencies)
    - npm: Add peerDependencies as called for by ESLint
    - Travis: Add Node 12/ESLint 6 and Node 10/ESLint 5 tests

Your Travis file tests ESLint 2 and your README insists on a minimum version of 4, so I wasn't sure which one to fix (and thus which to use in the peerDependencies field which ESLint calls for).

As far as Node version, FWIW, your devDeps import-from and eslint can both be updated to the latest versions if you move to a minimum version of Node 8.

@brettz9
Copy link
Contributor Author

brettz9 commented Oct 23, 2019

Are you going to have time to take a look?

@Turbo87
Copy link
Owner

Turbo87 commented Oct 23, 2019

you are doing a lot of different changes in this PR which makes it significantly harder to review. can you split them up into dedicated PRs where each PR only changes one specific thing? 🙏

@brettz9 brettz9 force-pushed the recommended-config branch 2 times, most recently from 2a8803d to a711e9a Compare October 24, 2019 00:21
@brettz9
Copy link
Contributor Author

brettz9 commented Oct 24, 2019

I've removed the ES6 changes which were the bulk of the PR. The number of changes in this PR are much smaller now; they are just a handful of small changes instead. Is that ok?

I can submit the refactoring PR separately which has more changes, though of the same type.

@Turbo87
Copy link
Owner

Turbo87 commented Oct 24, 2019

you still have a list of things that it does in the description. adjusting the CI config is a separate thing, doing stuff with package-lock.json is too, peerDependencies is one, etc.

@brettz9
Copy link
Contributor Author

brettz9 commented Oct 24, 2019

Ok, I've changed to just handle one item in this PR. I can submit the others separately. Please review this one if you would.

Copy link
Owner

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

could you add some documentation on how to enable this config in ESLint?

index.js Outdated Show resolved Hide resolved
@Turbo87 Turbo87 changed the title Recommended config and housecleaning Add recommended config Oct 24, 2019
@brettz9 brettz9 force-pushed the recommended-config branch 2 times, most recently from c70d8a7 to 9b00c0b Compare October 24, 2019 15:09
Copy link
Owner

@Turbo87 Turbo87 left a comment

Choose a reason for hiding this comment

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

👏

@Turbo87 Turbo87 merged commit 3ae6ba1 into Turbo87:master Oct 24, 2019
@brettz9 brettz9 deleted the recommended-config branch October 25, 2019 00:23
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.

Add recommended config
2 participants