feat: generate lint on new project and run lint on update project #149
Conversation
|
@dalssoft can you check this PR please? |
|
@mtperesvx I think the idea here is to add several lints with one as the default, not just add 1, right? |
|
actually we should run the lint (1) standard and (2) always (new and update) without asking. the only option should be deactivate it. |
|
@dalssoft so do these changes cover the requirements, right? In this PR:
|
| @@ -136,6 +142,7 @@ const cmd = { | |||
|
|
|||
| if (npmOptions.npmInstall === 'Yeah, please' || npmOptions.npmInstall === 'yes') { | |||
| await exec('npm install') | |||
| await exec('npx eslint \"**/*.{js,jsx}\" --fix') | |||
There was a problem hiding this comment.
if we run it without an eslint file configured, it will ask to install it.
so, if I said before (line 69) that i don't want to use eslint on my project and it(npx eslint) installs the eslint when I generate the project, we will provide a bad DX for programmer
We need to run it only if the project use eslint (when generated or after it)
| @@ -6,6 +6,7 @@ const cmd = { | |||
| alias: ['u'], | |||
| run: async toolbox => { | |||
| const generators = (await generator(toolbox)).update | |||
| await exec('npx eslint \"**/*.{js,jsx}\" --fix') | |||
There was a problem hiding this comment.
we need to check if exists an eslint file, if we run it without an eslint file, the npx eslit will ask to install and configure it.
it could generate a bad DX for the programmer
| @@ -7,6 +7,7 @@ const optionalPackages = { | |||
| mysql: ['"@herbsjs/herbs2knex": "^1.5.2"', '"mysql2": "^2.3.3"'], | |||
| rest: ['"express": "^4.18.1"', '"cors": "^2.8.5"', '"@herbsjs/herbs2rest": "^3.0.1"'], | |||
| graphql: ['"graphql": "^16.5.0"', '"@herbsjs/herbs2gql": "^2.2.0"', '"apollo-server": "^3.8.2"','"apollo-server-express": "^3.8.2"', '"graphql-tools": "^8.2.12"', '"graphql-scalars": "^1.17.0"',] | |||
| eslint: ['"eslint": "^8.20.0"', '"eslint-config-standard": "^17.0.0"', '"eslint-plugin-import": "^2.26.0","eslint-plugin-n": "^15.2.4"','"eslint-plugin-promise": "^6.0.0"'] | |||
There was a problem hiding this comment.
we don't generate code using import, are you sure we need the eslint-plugin-import pkg?
| env: { | ||
| es2021: true, | ||
| node: true | ||
| }, |
There was a problem hiding this comment.
why not just call the npx eslint after creating the project? so the developer could use the eslint CLI to generate it as they need/want to, we don't need to generate/manage it, just pass this job for eslint CLI
There was a problem hiding this comment.
Would it make sense to keep the lint --fix command as the responsibility of the herbs update command in this case?
There was a problem hiding this comment.
Only the lint, so we will warn the developer about the problems, run the lint --fix could be so much invasive for the developer, for some reason they may want to maintain the code with the "lint problems"
| @@ -136,6 +142,7 @@ const cmd = { | |||
|
|
|||
| if (npmOptions.npmInstall === 'Yeah, please' || npmOptions.npmInstall === 'yes') { | |||
| await exec('npm install') | |||
| await exec('npx eslint \"**/*.{js,jsx}\" --fix') | |||
There was a problem hiding this comment.
since herbs update will run the lint command, this line is not necessary for when the user chooses Yeap. However, it is necessary when the user chooses No.
|
Is this PR still active? |
|
@mtperesvx are you still working on it? |
|
@dalssoft Yes, I will go back to work on it. |
|
@matnperes your PR is under conflict |
No description provided.