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

New: pass cwd from cli engine #12389

Merged
merged 2 commits into from Oct 16, 2019
Merged

Conversation

@fa93hws
Copy link
Contributor

@fa93hws fa93hws commented Oct 8, 2019

What is the purpose of this pull request? (put an "X" next to 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:

What changes did you make? (Give an overview)
Pass the cwd from the cli engine
#11218
RFC link: eslint/rfcs#35

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

@eslint-deprecated eslint-deprecated bot added the triage label Oct 8, 2019
lib/linter/linter.js Outdated Show resolved Hide resolved
Copy link
Member

@kaicataldo kaicataldo left a comment

Thanks for working on this!

I think the new Linter config object should be optional so that it’s not a breaking change (and we don’t have to add that empty object everywhere).

Do you mind updating the documentation as well?

lib/linter/linter.js Outdated Show resolved Hide resolved
lib/linter/linter.js Outdated Show resolved Hide resolved
lib/linter/linter.js Outdated Show resolved Hide resolved
Copy link
Member

@mysticatea mysticatea left a comment

Thank you for your contribution!
I have some suggestions. Would you address those?

lib/linter/linter.js Outdated Show resolved Hide resolved
lib/linter/linter.js Outdated Show resolved Hide resolved
lib/linter/linter.js Outdated Show resolved Hide resolved
@fa93hws fa93hws requested review from kaicataldo and mysticatea Oct 9, 2019
Copy link
Member

@mysticatea mysticatea left a comment

Thank you for the quick update!

Almost LGTM, but I have a few suggestions.

docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
lib/linter/linter.js Outdated Show resolved Hide resolved
lib/linter/linter.js Outdated Show resolved Hide resolved
@fa93hws fa93hws requested a review from mysticatea Oct 10, 2019
Copy link
Member

@kaicataldo kaicataldo left a comment

Some suggestions around documentation. Otherwise, this LGTM. Thanks for working on this!

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/working-with-rules.md Outdated Show resolved Hide resolved
tests/lib/linter/linter.js Outdated Show resolved Hide resolved
docs/developer-guide/nodejs-api.md Outdated Show resolved Hide resolved
@kaicataldo
Copy link
Member

@kaicataldo kaicataldo commented Oct 10, 2019

Ah, also forget to mention: can you change the commit message from Chore: to Enhancement: New:? Since this adds a new API, it should be a semver-minor change.

Edited: @mysticatea Whoops, yes! Thank you for catching that.

@mysticatea
Copy link
Member

@mysticatea mysticatea commented Oct 10, 2019

@kaicataldo I guess you meant Update: or New:.

@kaicataldo kaicataldo added feature and removed enhancement labels Oct 10, 2019
@fa93hws fa93hws force-pushed the fa93hws:pass-cwd-from-cli-engine branch from ef4119c to aea3c0d Oct 11, 2019
@fa93hws fa93hws changed the title Chore: pass cwd from cli engine New: pass cwd from cli engine Oct 11, 2019
@fa93hws fa93hws requested a review from kaicataldo Oct 11, 2019
@fa93hws fa93hws force-pushed the fa93hws:pass-cwd-from-cli-engine branch from 1279c98 to be20d43 Oct 11, 2019
Copy link
Member

@kaicataldo kaicataldo left a comment

LGTM, thanks!

Copy link
Member

@mysticatea mysticatea left a comment

LGTM, thank you!

@kaicataldo kaicataldo merged commit 874fe16 into eslint:master Oct 16, 2019
16 checks passed
16 checks passed
Verify Files
Details
Test (ubuntu-latest, 8.x) Test (ubuntu-latest, 8.x)
Details
Test (ubuntu-latest, 10.x) Test (ubuntu-latest, 10.x)
Details
Test (ubuntu-latest, 12.x) Test (ubuntu-latest, 12.x)
Details
Test (windows-latest, 12.x) Test (windows-latest, 12.x)
Details
Test (macOS-latest, 12.x) Test (macOS-latest, 12.x)
Details
Browser Test
Details
commit-message PR title follows commit message guidelines
Details
continuous-integration Build #20191011.6 succeeded
Details
continuous-integration (Test on Node.js 10 (Linux)) Test on Node.js 10 (Linux) succeeded
Details
continuous-integration (Test on Node.js 12 (Linux)) Test on Node.js 12 (Linux) succeeded
Details
continuous-integration (Test on Node.js 12 (Windows)) Test on Node.js 12 (Windows) succeeded
Details
continuous-integration (Test on Node.js 12 (macOS)) Test on Node.js 12 (macOS) succeeded
Details
continuous-integration (Test on Node.js 8 (Linux)) Test on Node.js 8 (Linux) succeeded
Details
licence/cla Contributor License Agreement is signed.
Details
release-monitor No patch release is pending
Details
@kaicataldo
Copy link
Member

@kaicataldo kaicataldo commented Oct 16, 2019

Thanks for contributing!

@fa93hws fa93hws deleted the fa93hws:pass-cwd-from-cli-engine branch Oct 16, 2019
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Apr 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants