-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
feat: move eslint --init
to @eslint/create-config
#15150
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
Conversation
I think the files should be copied to the new repo https://github.com/eslint/create-config, as we decided not to go in the monorepo direction for this. |
absolutely right -- that's why the PR is a draft, will do later. 😄 |
update: I was planning to move proxyrequire to testdouble in |
eslint --init
to @eslint/create-configeslint --init
to @eslint/create-config
What’s the status on this? We are getting a bunch of issues/PRs related to —init and we need to make some progress. |
Now I'm using the native Node.js esm as sindresorhus' blog (not pushed yet):
additionally, I replaced does it make sense? Do we want to add the cjs export in the new package, too? |
c2744b3
to
7d5c492
Compare
Do we want to export anything from this package? If it's meant to be only a CLI app, I think we can publish ESM-only package, without |
I agree. I don’t think we need to export anything. |
7f69c98
to
a4ac601
Compare
Where are we on this? Is there a reason all the work is being done here instead of on the new repo? |
128c2e5
to
f09f525
Compare
this is ready for review, but set to a draft to make sure we don't merge it before |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was able to test this manually locally with this setup:
- In my
eslint/create-config
checkout,npm link
. - In my
eslint/eslint
checkout on this branch,npm link @eslint/create-config
. - In my
eslint/eslint
checkout on this branch,npx eslint --init
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks! Leaving as comment instead of approval pending release of @eslint/create-config
.
0515509
to
039eced
Compare
Hi @aladdin-add!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
To Fix: You can fix this problem by running Read more about contributing to ESLint here |
039eced
to
45c23f9
Compare
Where are we on this? What’s needed now? |
The only thing is: eslint/create-config#10. Given npx will always use latest, seems this one can even be merged first. |
Okay, just releases v0.1.2. See my note on the issue about the version number. |
Should we keep unit tests for the |
Can we delete |
fixes eslint#14768, fixes eslint#15159 Update: rm eslint auto config refs: https://github.com/aladdin-add/rfcs/blob/bdc12aa062750d837e5a3bbbf2f6e5e3a98da388/designs/2021-init-command-eslint-cli/README.md#1-remove-eslint-auto-config Update: mv eslint --init to another package moved `lib/init/config-rule` to tools, as it is also used by some others refs: https://github.com/aladdin-add/rfcs/blob/bdc12aa062750d837e5a3bbbf2f6e5e3a98da388/designs/2021-init-command-eslint-cli/README.md#2-move-eslint---init-related-files-to-a-separate-repo chore: fix imports to make test passing todo: use the new eslint api fix: use the new eslint api (async) fix: remove espree from deps chore: fix a failing test fix: a failing test chore: cleanup TODOs fix: allow to use local-installed eslint wip: fix one-var chore: lib => esm chore: tests => esm todo: proxyquire => td chore: update deps to latest fix: should write a file through fs when a ${fileType} path is passed replaced proxyquire & sinon => td fix: should include a newline character at EOF chore: add testdouble --wip-- [skip ci] chore: remove package @eslint/create-eslint feat: update npm --init to run `npm init @eslint/config` docs: update getting-started Update README.md Update getting-started.md chore: rm init fixtures fix: `npm init @eslint/config` output chore: rm unused files chore: rm unused deps Update bin/eslint.js Co-authored-by: Brandon Mills <btmills@users.noreply.github.com> chore: fix typo
7b932c4
to
82baad0
Compare
@mdjermanovic should be addressed now. 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Leaving open to allow others to follow up their reviews.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
TODOs:
Update: rm eslint auto config. rfc step 1
Update: mv eslint --init to another package. step 2
move to esm.
fix failing tests
move to the eslint/create-config repo
docs update
Prerequisites checklist
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
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
fixes #14768
Is there anything you'd like reviewers to focus on?