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 support for extends #163

Closed
wants to merge 5 commits into from
Closed

Add support for extends #163

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Sep 24, 2019

Closes #160
image
https://eslint.org/docs/developer-guide/nodejs-api


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


IssueHunt has been backed by the following sponsors. Become a sponsor

@sindresorhus
Copy link
Owner

It should be exposed as extends option, not baseConfig.extends.

readme.md Outdated Show resolved Hide resolved
@ghost ghost changed the title Add example of using the baseConfig option Add support for extends Sep 25, 2019
@ghost
Copy link
Author

ghost commented Sep 25, 2019

@sindresorhus If desired, all of the baseConfig options can be hoisted by using ESLint's config schema: https://github.com/sindresorhus/grunt-eslint/compare/master...pro-src:hoist_base_config?expand=1
https://github.com/eslint/eslint/blob/master/conf/config-schema.js

And maybe fail hard if an unrecognized option is specified.

@ghost
Copy link
Author

ghost commented Sep 29, 2019

@sindresorhus,

I know you have 1k other PRs to like at that are far more interesting but I'm bored so I'm going to bug you anyway. 😜

I'm wondering if you had something else in mind for this PR. As the the issue title is confusing to me. Simply because there is already support for the "extends" option. I get what you mean by it should be exposed as "extends" but it has already been exposed as "extends" by ESLint though nested under the documented baseConfig option. I've taken a guess at what the expectation is for this, made changes, and made some suggestions. So, I don't really know what else to do with it. Also, If you just didn't realize that it was already exposed, feel free to kick this out of limbo and into the trash. I don't mind. 😄

Thanks

@sindresorhus
Copy link
Owner

Nah, the point is to just make the extends option more discoverable and easier to use by making it top-level and documenting it.

--

I get what you mean by it should be exposed as "extends" but it has already been exposed as "extends" by ESLint though nested under the documented baseConfig option.

Yes, but it's not discoverable.

readme.md Outdated Show resolved Hide resolved
tasks/eslint.js Outdated Show resolved Hide resolved
tasks/eslint.js Outdated Show resolved Hide resolved
tasks/eslint.js Outdated Show resolved Hide resolved
readme.md Outdated Show resolved Hide resolved
@ghost
Copy link
Author

ghost commented Sep 30, 2019

@kevva Thanks for the review. Did you notice https://github.com/sindresorhus/grunt-eslint/compare/master...pro-src:hoist_base_config?expand=1 ?

It uses the rest operator 👍

@ghost ghost closed this Sep 30, 2019
@ghost ghost deleted a comment Sep 30, 2019
This pull request was closed.
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.

Add support for extends
2 participants