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

refactor(find): Use new eslint getRules() api #237

Merged
merged 1 commit into from May 5, 2017

Conversation

randycoulman
Copy link
Contributor

Breaking change: Requires eslint >= 3.12.0

Starting with version 3.12.0, eslint provides a getRules() API that allows us to directly access the rules rather than trying to load them from the file system. This is cleaner, eliminates a dependency on the internal structure of the eslint codebase.

This change paves the way for further features involving deprecated rules, such as #172 and #188.

Note that eslint returns a Map object containing the rules, so we can’t use Object.keys() to get the keys out.

We are not yet using the rule objects themselves, so the test stubs just return empty objects for now.

Closes #211.

Breaking change: Requires eslint >= 3.12.0

Starting with version 3.12.0, eslint provides a getRules()
API that allows us to directly access the rules rather than
trying to load them from the file system.  This is cleaner,
eliminates a dependency on the internal structure of the eslint
codebase, and may even be faster.

This change paves the way for further features involving
deprecated rules, such as sarbbottam#172 and sarbbottam#188.

Closes sarbbottam#211.
@codecov-io
Copy link

Codecov Report

Merging #237 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #237   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         216    209    -7     
=====================================
- Hits          216    209    -7
Impacted Files Coverage Δ
src/lib/rule-finder.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd2b370...9f94572. Read the comment docs.

Copy link
Collaborator

@ta2edchimp ta2edchimp left a comment

Choose a reason for hiding this comment

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

Thanks for your work!

@ta2edchimp
Copy link
Collaborator

I'll leave it open until this evening (EU TZ) to give another chance for feedback. Otherwise, I'm going to squash and merge this around 1600 UTC.
cc @sarbbottam @ljharb

@randycoulman
Copy link
Contributor Author

Awesome! I can't wait for this to get merged. But remember when releasing, it's a breaking change due to the need for ESLint >= 3.12.0!

@ta2edchimp ta2edchimp merged commit 7a4de6c into sarbbottam:master May 5, 2017
@ta2edchimp
Copy link
Collaborator

I'd personally prefer to also include #172 in a coming new (major version) release.

@randycoulman
Copy link
Contributor Author

@ta2edchimp That sounds like a good plan. I'll start working on it as soon as I can, but it might be a week before I can get to it. If someone else wants to run with it, feel free, but let me know so I don't duplicate effort.

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.

None yet

4 participants