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 label Feb 19, 2020
@eslint eslint deleted a comment from jsf-clabot Feb 19, 2020
@kaicataldo kaicataldo added core do not merge enhancement and removed triage 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 Mar 6, 2020
@nzakas nzakas added this to Implemented, pending review in v7.0.0 Mar 26, 2020
@kaicataldo
Copy link
Member Author

kaicataldo commented Apr 9, 2020

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 Apr 9, 2020
mysticatea and others added 5 commits Apr 23, 2020
Co-Authored-By: Nicholas C. Zakas <nicholas@nczconsulting.com>
Co-Authored-By: Kai Cataldo <kai@kaicataldo.com>
@mysticatea
Copy link
Member

mysticatea commented Apr 23, 2020

Thank you for your review! I applied the suggestions.

@mysticatea mysticatea removed the do not merge 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

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

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 Apr 24, 2020
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

mysticatea commented Apr 24, 2020

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

Copy link
Member

@btmills btmills left a comment

LGTM. Again, this is great work 👏

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

kaicataldo commented Apr 24, 2020

@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 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 core enhancement
Projects
No open projects
v7.0.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants