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

Update: Add ESLint API (refs eslint/rfcs#40) #12939

Merged
merged 73 commits into from Apr 24, 2020
Merged

Update: Add ESLint API (refs eslint/rfcs#40) #12939

merged 73 commits into from Apr 24, 2020

Conversation

kaicataldo
Copy link
Member

@kaicataldo kaicataldo commented Feb 19, 2020

Prerequisites checklist

  • I have read the contributing guidelines.
  • The team has reached consensus on the changes proposed in this pull request. If not, I understand that the evaluation process will begin with this pull request and won't be merged until the team has reached consensus.

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

refs eslint/rfcs#40

This is a WIP. I received some initial feedback here and have incorporated that into this PR.

What changes did you make? (Give an overview)

This PR adds the new ESLint class, which is an asynchronous wrapper around the CLIEngine class.

Is there anything you'd like reviewers to focus on?

Still a WIP, but as I get closer to completion: are the options/type defs correct?

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Feb 19, 2020
@eslint eslint deleted a comment from jsf-clabot Feb 19, 2020
@kaicataldo kaicataldo added core Relates to ESLint's core APIs and features do not merge This pull request should not be merged yet enhancement This change enhances an existing feature of ESLint and removed triage An ESLint team member will look at this issue soon labels Feb 19, 2020
lib/eslint/eslint.js Outdated Show resolved Hide resolved
@kaicataldo kaicataldo force-pushed the eslint-api branch 2 times, most recently from d394aa6 to d7276fd Compare March 6, 2020 02:30
@nzakas nzakas added this to Implemented, pending review in v7.0.0 Mar 26, 2020
@kaicataldo
Copy link
Member Author

Closing this for now while I’m out sick, in case someone else can get to it first.

@kaicataldo kaicataldo closed this Apr 9, 2020
@kaicataldo kaicataldo removed this from Implemented, pending review in v7.0.0 Apr 9, 2020
@nzakas
Copy link
Member

nzakas commented Apr 9, 2020

Reopening in case someone wants to work off of this branch (and so it doesn't get removed from the project board).

@nzakas nzakas reopened this Apr 9, 2020
@btmills btmills added this to Accepted, ready to implement in v7.0.0 Apr 9, 2020
@nzakas nzakas marked this pull request as draft April 9, 2020 20:40
mysticatea and others added 5 commits April 24, 2020 04:13
Co-Authored-By: Nicholas C. Zakas <nicholas@nczconsulting.com>
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
@mysticatea
Copy link
Member

Thank you for your review! I applied the suggestions.

@mysticatea mysticatea removed the do not merge This pull request should not be merged yet label Apr 23, 2020
@btmills btmills moved this from Accepted, ready to implement to Implemented, pending review in v7.0.0 Apr 23, 2020
Copy link
Member Author

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

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

Doesn't look like I can officially approve this (maybe because I'm also an author), but LGTM 👍

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

Bravo to both @kaicataldo and @mysticatea 👏 This was an enormous lift, and I'm excited to be able to use the new API. I had some suggestions, mostly around the documentation.

docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
lib/cli-engine/cli-engine.js Show resolved Hide resolved
lib/cli-engine/cli-engine.js Outdated Show resolved Hide resolved
tests/lib/cli.js Outdated Show resolved Hide resolved
mysticatea and others added 11 commits April 24, 2020 11:11
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
Co-Authored-By: Brandon Mills <btmills@users.noreply.github.com>
@mysticatea
Copy link
Member

Thank you for your review! I applied suggestions to this PR.

Copy link
Member

@btmills btmills left a comment

Choose a reason for hiding this comment

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

LGTM. Again, this is great work 👏

@kaicataldo kaicataldo merged commit bcafd0f into master Apr 24, 2020
v7.0.0 automation moved this from Implemented, pending review to Done Apr 24, 2020
@kaicataldo kaicataldo deleted the eslint-api branch April 24, 2020 17:39
@kaicataldo
Copy link
Member Author

@mysticatea Thank you again for picking this up while I was sick ❤️

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Oct 22, 2020
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Oct 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
No open projects
v7.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants