From 3464fb1c5389ab7cf8efd74e0d720cb943d4ab06 Mon Sep 17 00:00:00 2001 From: Anix Date: Sun, 28 Jun 2020 20:57:59 +0530 Subject: [PATCH 1/3] Update: move init command to eslint-cli --- .../2020-init-command-eslint-cli/README.md | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) create mode 100644 designs/2020-init-command-eslint-cli/README.md diff --git a/designs/2020-init-command-eslint-cli/README.md b/designs/2020-init-command-eslint-cli/README.md new file mode 100644 index 00000000..35a8af13 --- /dev/null +++ b/designs/2020-init-command-eslint-cli/README.md @@ -0,0 +1,90 @@ +- Repo: [eslint/eslint-cli](https://github.com/eslint/eslint-cli), [eslint/eslint](https://github.com/eslint/eslint) +- Start Date: 2020-06-29 +- RFC PR: +- Authors: Aniketh Saha ([anikethsaha](https://github.com/anikethsaha)) + +# init command in `eslint-cli` + +## Summary + +Move the `init` command from main repo ([eslint](https://github.com/eslint/eslint)) to [eslint-cli](https://github.com/eslint/eslint-cli) repo + +## Motivation + +Currently the whole `init` command is being shipped with the cli in main repo. Though its not that much of size (roughly ~0.6MB for the [eslint/lib/init](https://github.com/eslint/eslint/tree/master/lib/init)), this command is not a type of command that we need +everyday or everytime running eslint. It is mainly used when creating a new project or adding eslint to a project for first time. So if we move this to a separate package that is meant to use in command line, +We will make use of tools like `npx` to simply run `npx eslint-cli init` single time for a project instead of having it in the core project. + +## Detailed Design + + + +The implementation is mainly migrating the code from main repo to `eslint-cli`. As far as the modules are concerned, all shared modules are being shipped currently with the eslint so we can use those modules through eslint itself. +As far as the `eslint-recommended` rules are concerned, it will be done as a copy paste to the `eslint-cli` repo. `eslint-cli` has two binary names as `eslint` and `eslint-cli`, it will be changed to `eslint-cli` alone in order to avoid any conflicts between two packages and command name. + +## Documentation + + +Documentation for `eslint-cli` will be updated with proper usage and each prompt's details. In the main documentaion, a link to `eslint-cli` will be given and basic usage and the package details will be written. Like this +Yes as it will be a major change for the cli, I guess a blog can be a good solution to solve inital queries. + +## Drawbacks + + +This might increase maintenance burden as currently `eslint-cli` is quite inactive but I believe it will be quite same as this is just moving a feature. +Might not be benificial for users who installed `eslint` at a global scope. + + +## Backwards Compatibility Analysis + + +Releasing with a proper message which the command is being run from `eslint` with something like - `"this command has been moved from here, use 'npx eslint-cli init'"` might be a good way to address backward compatibility. +Otherwise we can keep the feature in the main repo as well for starting few days/weeks with a warning and then removing it completing with an error message. + +## Alternatives + + +The current implementation in `eslint` repo is the alternative. No changes needed. + +## Open Questions + + + +As `eslint-cli` repo is quite outdated in terms of dependencies, I think it would be better to update and migrate in a separate PR ? From 6e1ba18915b7f19d945ac0d822446835bb865b79 Mon Sep 17 00:00:00 2001 From: Anix Date: Wed, 5 Aug 2020 19:02:38 +0530 Subject: [PATCH 2/3] Update: changed the package name and open question --- designs/2020-init-command-eslint-cli/README.md | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/designs/2020-init-command-eslint-cli/README.md b/designs/2020-init-command-eslint-cli/README.md index 35a8af13..3759d4a2 100644 --- a/designs/2020-init-command-eslint-cli/README.md +++ b/designs/2020-init-command-eslint-cli/README.md @@ -7,13 +7,13 @@ ## Summary -Move the `init` command from main repo ([eslint](https://github.com/eslint/eslint)) to [eslint-cli](https://github.com/eslint/eslint-cli) repo +Move the `init` command from main repo ([eslint](https://github.com/eslint/eslint)) to a new repo named `@eslint/create` ## Motivation Currently the whole `init` command is being shipped with the cli in main repo. Though its not that much of size (roughly ~0.6MB for the [eslint/lib/init](https://github.com/eslint/eslint/tree/master/lib/init)), this command is not a type of command that we need everyday or everytime running eslint. It is mainly used when creating a new project or adding eslint to a project for first time. So if we move this to a separate package that is meant to use in command line, -We will make use of tools like `npx` to simply run `npx eslint-cli init` single time for a project instead of having it in the core project. +We will make use of tools like `npx` to simply run `npx @eslint/create` or using `npm` to run `npm init @eslint` or using `yarn` to run `yarn create @eslint` single time for a project instead of having it in the core project. ## Detailed Design @@ -27,7 +27,7 @@ We will make use of tools like `npx` to simply run `npx eslint-cli init` single --> The implementation is mainly migrating the code from main repo to `eslint-cli`. As far as the modules are concerned, all shared modules are being shipped currently with the eslint so we can use those modules through eslint itself. -As far as the `eslint-recommended` rules are concerned, it will be done as a copy paste to the `eslint-cli` repo. `eslint-cli` has two binary names as `eslint` and `eslint-cli`, it will be changed to `eslint-cli` alone in order to avoid any conflicts between two packages and command name. +As far as the `eslint-recommended` rules are concerned, it will be done as a copy paste to the `@eslint/create` repo. ## Documentation @@ -35,8 +35,8 @@ As far as the `eslint-recommended` rules are concerned, it will be done as a cop How will this RFC be documented? Does it need a formal announcement on the ESLint blog to explain the motivation? --> -Documentation for `eslint-cli` will be updated with proper usage and each prompt's details. In the main documentaion, a link to `eslint-cli` will be given and basic usage and the package details will be written. Like this -Yes as it will be a major change for the cli, I guess a blog can be a good solution to solve inital queries. +Documentation for `@eslint/create` will be created with proper usage and each prompt's details. In the main documentaion, a link to `@eslint/create` will be given and basic usage and the package details will be written. Like this +Yes as it will be a major change for the `eslint` repo itself as it is being remove from the main package/repo, I guess a blog can be a good solution to solve inital queries. ## Drawbacks @@ -50,8 +50,7 @@ Yes as it will be a major change for the cli, I guess a blog can be a good solut experience, etc. Try to identify as many potential problems with implementing this RFC as possible. --> -This might increase maintenance burden as currently `eslint-cli` is quite inactive but I believe it will be quite same as this is just moving a feature. -Might not be benificial for users who installed `eslint` at a global scope. +This might increase maintenance burden and if we are planing to have a monorepo, it might be a more burden with a requirement of proper tooling. ## Backwards Compatibility Analysis @@ -61,7 +60,7 @@ Might not be benificial for users who installed `eslint` at a global scope. change for them? If so, how are you going to minimize the disruption to existing users? --> -Releasing with a proper message which the command is being run from `eslint` with something like - `"this command has been moved from here, use 'npx eslint-cli init'"` might be a good way to address backward compatibility. +Releasing with a proper message which the command is being run from `eslint` with something like - `"this command has been moved from here, use 'npx @eslint/create'"` might be a good way to address backward compatibility. Otherwise we can keep the feature in the main repo as well for starting few days/weeks with a warning and then removing it completing with an error message. ## Alternatives @@ -87,4 +86,4 @@ The current implementation in `eslint` repo is the alternative. No changes neede you can remove this section. --> -As `eslint-cli` repo is quite outdated in terms of dependencies, I think it would be better to update and migrate in a separate PR ? +Do we want to implement this as monorepo under `eslint` repo or a separate repo `@eslint/create` but github will make it as `eslint-create` or similar ? From 35dc412f24f8c344327f7a03d4ac7f5ffc0c3a72 Mon Sep 17 00:00:00 2001 From: Anix Date: Sun, 9 Aug 2020 17:48:15 +0530 Subject: [PATCH 3/3] Update: added a detail explanation for the design --- .../2020-init-command-eslint-cli/README.md | 52 +++++++++++++++---- 1 file changed, 41 insertions(+), 11 deletions(-) diff --git a/designs/2020-init-command-eslint-cli/README.md b/designs/2020-init-command-eslint-cli/README.md index 3759d4a2..eab5d398 100644 --- a/designs/2020-init-command-eslint-cli/README.md +++ b/designs/2020-init-command-eslint-cli/README.md @@ -1,9 +1,9 @@ -- Repo: [eslint/eslint-cli](https://github.com/eslint/eslint-cli), [eslint/eslint](https://github.com/eslint/eslint) +- Repo: [eslint/eslint](https://github.com/eslint/eslint) - Start Date: 2020-06-29 - RFC PR: - Authors: Aniketh Saha ([anikethsaha](https://github.com/anikethsaha)) -# init command in `eslint-cli` +# Move --init flag into a separate utility ## Summary @@ -11,8 +11,7 @@ Move the `init` command from main repo ([eslint](https://github.com/eslint/eslin ## Motivation -Currently the whole `init` command is being shipped with the cli in main repo. Though its not that much of size (roughly ~0.6MB for the [eslint/lib/init](https://github.com/eslint/eslint/tree/master/lib/init)), this command is not a type of command that we need -everyday or everytime running eslint. It is mainly used when creating a new project or adding eslint to a project for first time. So if we move this to a separate package that is meant to use in command line, +Currently the whole `init` command is being shipped with the cli in the main repo. Though its not that much of size (roughly ~0.6MB for the [eslint/lib/init](https://github.com/eslint/eslint/tree/master/lib/init)), this command is not a type of command that we need everyday or everytime running eslint. It is mainly used when creating a new project or adding eslint to a project for the first time. So if we move this to a separate package that is meant to use in the command line, We will make use of tools like `npx` to simply run `npx @eslint/create` or using `npm` to run `npm init @eslint` or using `yarn` to run `yarn create @eslint` single time for a project instead of having it in the core project. ## Detailed Design @@ -26,8 +25,35 @@ We will make use of tools like `npx` to simply run `npx @eslint/create` or using used. Be sure to define any new terms in this section. --> -The implementation is mainly migrating the code from main repo to `eslint-cli`. As far as the modules are concerned, all shared modules are being shipped currently with the eslint so we can use those modules through eslint itself. -As far as the `eslint-recommended` rules are concerned, it will be done as a copy paste to the `@eslint/create` repo. +The implementation is mainly migrating the code from main repo to `@eslint/create`. As far as the modules are concerned, all shared modules are being shipped currently with the eslint so we can use those modules through eslint itself. + +The approach would be following these steps + +- Move the `eslint/lib/init/*` to the new repo's `src/*` directory +- Add the following `dependencies` in `package.json` + - `enquirer` + - `progress` + - `semver` + - `espree` (peer) + - `lodash` + - `json-stable-stringify-without-jsonify` + - `cross-spawn` + - `eslint` (peer) + - `debug` (peer) +- for all those modules coming inside `../**/*`, it would be replaced using `eslint/**/*` +- For `tests`, move the `tests/lib/init` to new repo's `tests/init` +- Add the following `devDependencies` in `package.json` + - `chai` + - `sinon` + - `js-yaml` + - `proxyquire` + - `shelljs` +- create `tests/utils` and add the following functions + - [`defineInMemoryFs`](https://github.com/eslint/eslint/blob/6677180495e16a02d150d0552e7e5d5f6b77fcc5/tests/_utils/in-memory-fs.js#L250) +- for `conf/*`, it is not a part of the public API and it cant be accessed from `eslint` module, we need a keep these synced with the main repo (`eslint`). We can choose from the following approach to solve this : + - using the `eslint-github-bot`, whenever there is a PR opened that changes the file for the `conf` directory in `eslint` repo, create an issue in the `@eslint/create` repo to have a look in the PR and if required submit a change in `@eslint/create`. + - OR, while doing the release for both `eslint` and `@eslint/create` repos, the release script can be configured to copy-parse the `conf` folder from `eslint` to `@eslint/create` before the release just like the `website` repo is being updated currently. +- After completion of this repo, whenever someone uses the `--init` flag, an error will be thrown stating the correct step to do the migration i.e using `npx @eslint/create` or `npm @eslint` or `yarn @eslint`. I am not suggesting to use run the `@eslint/create` module whenever `--init` command is being called because this would require to use the `@eslint/create` module as a dependency in `eslint` and in `@eslint/create` we are already being using `eslint` as a dependency, so there would be circular dependencies. ## Documentation @@ -35,8 +61,9 @@ As far as the `eslint-recommended` rules are concerned, it will be done as a cop How will this RFC be documented? Does it need a formal announcement on the ESLint blog to explain the motivation? --> -Documentation for `@eslint/create` will be created with proper usage and each prompt's details. In the main documentaion, a link to `@eslint/create` will be given and basic usage and the package details will be written. Like this -Yes as it will be a major change for the `eslint` repo itself as it is being remove from the main package/repo, I guess a blog can be a good solution to solve inital queries. +Documentation for `@eslint/create` will be created with proper usage and each prompt's details. In the main documentation, a link to `@eslint/create` will be given [here](https://github.com/eslint/eslint/blob/master/docs/user-guide/command-line-interface.md#--init) and basic usage and the package details will be documented below as a deprecated notice and migration guide. +Also in the cli command options, [here](https://github.com/eslint/eslint/blob/master/docs/user-guide/command-line-interface.md#options), for `--init` we need to change the description for this with deprecated details as the command is still active just that there will be error that will recommend to use `@eslint/create` package instead. +Yes as it will be a major change for the `eslint` repo itself as it is being removed from the main package/repo, I guess a blog can be a good solution to solve initial queries. ## Drawbacks @@ -50,7 +77,9 @@ Yes as it will be a major change for the `eslint` repo itself as it is being rem experience, etc. Try to identify as many potential problems with implementing this RFC as possible. --> -This might increase maintenance burden and if we are planing to have a monorepo, it might be a more burden with a requirement of proper tooling. +- This might increase the maintenance burden as this will add one more repo in the organization. +- We may need to face challenges like re-directing of issues from `@eslint/create` repo to `eslint` and vice-versa. +- If we do not add proper tooling for handling the `conf` directory in `eslint` to `@eslint/create` repo as mentioned in the [detailed design](@detailed_design)'s 7th point, there will be an overhead work on looking through each PR/change particularly to find out whether there is a need for updating the `conf` directory in `@eslint/create` or not. ## Backwards Compatibility Analysis @@ -60,8 +89,9 @@ This might increase maintenance burden and if we are planing to have a monorepo, change for them? If so, how are you going to minimize the disruption to existing users? --> -Releasing with a proper message which the command is being run from `eslint` with something like - `"this command has been moved from here, use 'npx @eslint/create'"` might be a good way to address backward compatibility. -Otherwise we can keep the feature in the main repo as well for starting few days/weeks with a warning and then removing it completing with an error message. +- Releasing with a proper message which the command is being run from `eslint` with something like - `"this command has been moved from here, use 'npx @eslint/create'"` might be a good way to address backward compatibility. +- Or whenever `--init` command is being used, show a warning and run `npx @eslint/create` using `child_process` of the native node modules. +- Otherwise we can keep the feature in the main repo as well for starting few days/weeks with a warning and then removing it completing with an error message but this would lead to circular dependencies and that is not an ideal case for managing dependencies, so I think we should remove this feature completely with an error message as mentioned above. ## Alternatives