teammates icon indicating copy to clipboard operation
teammates copied to clipboard

Linting should be skipped for generated files during local build and test

Open fsgmhoward opened this issue 3 years ago • 6 comments

This is a very minor improvement which would improve local testing efficiency in my opinion.

Steps to reproduce

Run npm lint (specifically, it affects lint:css and lint:space).

Basically, the auto generated files of npm run build has been linted, resulting a large number of errors thrown out. Also, json files generated by lnp tests also fails the test.

Expected behaviour

Only linting whatever to be pushed to the git repository, skipping all the auto-generated items (such as those listed in .gitignore).

Additional info

For the lint:css, issue is resolved by adding --ignore-pattern \"src/web/dist/*.css\" behind the current command line strings. This instructs the stylelint not to check files in dist folder (the generated ones for production).

For the lint:space, lintspaces program does not have a good way to skip certain files. Probably a better matching rules needed to match files for linting.

fsgmhoward avatar Feb 11 '22 12:02 fsgmhoward

This may be somewhat related, but the better solution may be to just lint files that have diff. Currently, npm run lint lints all typescript/html files, which is unnecessarily long. This method will solve this issue and also improve the efficiency of linting.

FergusMok avatar Apr 01 '22 12:04 FergusMok

Hello, is anybody investigating this issue? If not I don't mind looking into it!

tenebrius1 avatar Jul 20 '22 15:07 tenebrius1

@tenebrius1 I don't think anyone is doing that. Please proceed.

fsgmhoward avatar Jul 21 '22 02:07 fsgmhoward

@fsgmhoward I have a few questions regarding this issue:

  1. I'm not too sure what the team's stance is with regards to solutions that add in new dependencies so I just wanted to check if it is okay if a solution I found requires adding in a new NPM package -- globby as suggested in this issue of the lintspaces-cli repo. I have tested this package and it allows for negated patterns e.g. "!src/web/dist/*" which would effectively exclude all the build files in the dist folder.
  2. Are there any specific files/path that you are looking to exclude for the lintspaces program? So far I can only identify the files in src/web/dist as build files to be excluded. Could you point me to any documentation that mentions where the build files are being generated?

tenebrius1 avatar Jul 22 '22 15:07 tenebrius1

@tenebrius1

  1. As far as I know we are quite loose about npm dependencies since they are used frontend and do not impose much security issue to the production data. You may proceed if you think it is necessary
  2. I do not have a list of them. However, as @FergusMok suggested above, you may actually consider making it to lint files with git diff only. This will effectively skips files unchanged and those should be excluded (e.g. those in .gitignore).

fsgmhoward avatar Jul 26 '22 02:07 fsgmhoward

@fsgmhoward Thanks for the clarification! I will look into whether there is a way to only run linting on files with git diff.

tenebrius1 avatar Jul 26 '22 08:07 tenebrius1