docs icon indicating copy to clipboard operation
docs copied to clipboard

fix: resolve precommit issue

Open ashutosh-rath02 opened this issue 2 years ago • 9 comments

Fixes #99 Added lint-staged to package.json as lint-staged was failing during commit Removed cspell as the flag of cspell was raised even if there was no error. @adithyaakrishna Please look into it.

ashutosh-rath02 avatar Jun 19 '23 12:06 ashutosh-rath02

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


Commit c6311aec7723d21e150ae4fa4166e07868301d66

Key changes:

  • Removed the cspell command from .lintstagedrc.json file.
  • Added the lint-staged command in the package.json file.

Potential problems:

  • The reason for removing the cspell command is not mentioned in the Pull Request. It would be useful to know whether this was an intentional change and if it affected the project's functionality in any way.
  • It is unclear why the lint-staged command was added to the package.json file. There is no explanation in the Pull Request.

alabulei1 avatar Jun 19 '23 12:06 alabulei1

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json

It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM

The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

adithyaakrishna avatar Jun 19 '23 13:06 adithyaakrishna

Also lint check seems to fail here, https://github.com/WasmEdge/docs/actions/runs/5312158635/jobs/9616295701?pr=115 👀

adithyaakrishna avatar Jun 19 '23 13:06 adithyaakrishna

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json

It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM

The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint?

ashutosh-rath02 avatar Jun 20 '23 12:06 ashutosh-rath02

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint? "lint-staged": { '*.(js|ts){,x}': 'eslint --cache --fix --ignore-path .gitignore', '*.{js,ts,tsx,css,md,html}': 'prettier --write', } @adithyaakrishna

ashutosh-rath02 avatar Jun 20 '23 14:06 ashutosh-rath02

Also lint check seems to fail here, https://github.com/WasmEdge/docs/actions/runs/5312158635/jobs/9616295701?pr=115 👀

Here, I guess I had to run prettier --check

ashutosh-rath02 avatar Jun 20 '23 14:06 ashutosh-rath02

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint?

Look at the suggested changes its already ensuring eslint runs if .ts or .tsx files is changed here you can add js and jsx file here additionally

duckling69 avatar Jun 24 '23 05:06 duckling69

@Lucif3r-in I dont think this scrip will work, lint-staged should be seperated from scripts in package.json It should be something like this, Screenshot 2023-06-19 at 6 58 48 PM The above is just an example, more info: https://www.npmjs.com/package/lint-staged & https://github.com/okonet/lint-staged

How do I make sure what files to check for eslint?

Look at the suggested changes its already ensuring eslint runs if .ts or .tsx files is changed here you can add js and jsx file here additionally

I think this is not a suggested change. It was an example, we have to look at the codebase and make changes accordingly. I am working on it. Thanks

ashutosh-rath02 avatar Jun 24 '23 05:06 ashutosh-rath02

Hi @ashutosh-rath02

Please check the CI checks. The Lint failed. Thanks.

alabulei1 avatar Jan 25 '24 11:01 alabulei1