6 ms·

Yes, because you lint everything in CI. Otherwise, linter warnings will start creeping into your codebase immediately, and the tool becomes much less useful.

mathverse 3y ago
Would not you lint only on files that changed?
- erikaww 3y ago
  I'm not sure if Eslint has this, but there could be cross-file lints (eg. unused variables). If some file changes, you may need to relint dependencies and dependent files. This could recursively trickle. I'm not sure if Eslint does this either, but indices or some incremental static analysis sounds like it could help the linter minimize rechecks or use previous state.
  - msoad 3y ago
    if you have one file that every single file across the repo imports in a way and you make changes to that file, you might run the linter for the entire repo. But again, how likely is this scenario?
    - erikaww 3y ago
      If the index or incremental static analysis object was designed well enough, I don't think you would need to lint every file, you would just need to look at files that consume that variable. Maybe you would look at every index? I'm not sure how well this could scale across (600- 1000?) different lints though. I should look into static analysis a bit more.
  - ehutch79 3y ago
    You can tell eslint about globals in it's config. But if you're using variables that arn't declared in the file somehow, that might be an issue you want to look at in general. That's a potential foot gun a linter should be balking at.
- rwilsonperkin 3y ago
  As the sibling comment mentions, you may have lint rules that depend on checking for the existence of, or properties of, another file. A popular set of rules comes from https://www.npmjs.com/package/eslint-plugin-import https://www.npmjs.com/package/eslint-plugin-import which validates imports, prevents circular dependencies, etc
- indymike 3y ago
  74 minutes of linting vs 1.3 seconds of linting? If a file has been linted, is unchanged since it was linted, there's literally no need to lint it again. Much like if you only need to process one record, you don't query the whole table to get the record.
  - AndrewDucker 3y ago
    File A depends on File B. File B moves. File A is now wrong, because it is unchanged.
    - indymike 3y ago
      Static analysis != linter.
sanitycheck 3y ago
I think if my CI was taking 45 mins to lint I'd look at linting only the files changed since the previous build instead of splitting it across 40+ workers. Or writing a new linter in Rust. But I'm generally working in a (human & financially) resource-constrained environment.
- throwup238 3y ago
  Typescript lints are type-aware so you can’t just lint changed files, you have to relint the entire codebase to check if any type changes have impacted the unchanged code.
  - pcthrowaway 3y ago
    Wouldn't an issue with a type change be caught at typescript compile/check steps? I'm not aware of eslint rules which would complain about some other untouched file if types have changed in ways such that the program still compiles
    - anamexis 3y ago
      A few examples of typescript-eslint rules that could fail when a type in another file is changed: https://typescript-eslint.io/rules/await-thenable/ https://typescript-eslint.io/rules/await-thenable/ https://typescript-eslint.io/rules/no-for-in-array https://typescript-eslint.io/rules/no-for-in-array https://typescript-eslint.io/rules/no-duplicate-type-constituents https://typescript-eslint.io/rules/no-duplicate-type-constit...
  - Too 3y ago
    Is there no incremental lint mode? When developing you need that for instant feedback, same mechanism should work for CI.
- arp242 3y ago
  One problem is that a change in a.js may trigger a new error in b.js. ESLint could also cache things fairly trivially: hash = hash_file_contents() if previously_seen_hashes.contains(hash) report_previous_results() else run_lint_and_cache_results() end maybe that already exists. But that has the same problem. When you've got enough hardware to throw at it, then "just run it on the full code" is the safest.
msoad 3y ago
I thought it would be obvious that in large codebases you only lint changed files in CI

mathverse 3y ago

Would not you lint only on files that changed?

erikaww 3y ago
I'm not sure if Eslint has this, but there could be cross-file lints (eg. unused variables). If some file changes, you may need to relint dependencies and dependent files. This could recursively trickle. I'm not sure if Eslint does this either, but indices or some incremental static analysis sounds like it could help the linter minimize rechecks or use previous state.
- msoad 3y ago
  if you have one file that every single file across the repo imports in a way and you make changes to that file, you might run the linter for the entire repo. But again, how likely is this scenario?
  - erikaww 3y ago
    If the index or incremental static analysis object was designed well enough, I don't think you would need to lint every file, you would just need to look at files that consume that variable. Maybe you would look at every index? I'm not sure how well this could scale across (600- 1000?) different lints though. I should look into static analysis a bit more.
- ehutch79 3y ago
  You can tell eslint about globals in it's config. But if you're using variables that arn't declared in the file somehow, that might be an issue you want to look at in general. That's a potential foot gun a linter should be balking at.
rwilsonperkin 3y ago
As the sibling comment mentions, you may have lint rules that depend on checking for the existence of, or properties of, another file. A popular set of rules comes from https://www.npmjs.com/package/eslint-plugin-import https://www.npmjs.com/package/eslint-plugin-import which validates imports, prevents circular dependencies, etc
indymike 3y ago
74 minutes of linting vs 1.3 seconds of linting? If a file has been linted, is unchanged since it was linted, there's literally no need to lint it again. Much like if you only need to process one record, you don't query the whole table to get the record.
- AndrewDucker 3y ago
  File A depends on File B. File B moves. File A is now wrong, because it is unchanged.
  - indymike 3y ago
    Static analysis != linter.

sanitycheck 3y ago

I think if my CI was taking 45 mins to lint I'd look at linting only the files changed since the previous build instead of splitting it across 40+ workers. Or writing a new linter in Rust. But I'm generally working in a (human & financially) resource-constrained environment.

throwup238 3y ago
Typescript lints are type-aware so you can’t just lint changed files, you have to relint the entire codebase to check if any type changes have impacted the unchanged code.
- pcthrowaway 3y ago
  Wouldn't an issue with a type change be caught at typescript compile/check steps? I'm not aware of eslint rules which would complain about some other untouched file if types have changed in ways such that the program still compiles
  - anamexis 3y ago
    A few examples of typescript-eslint rules that could fail when a type in another file is changed: https://typescript-eslint.io/rules/await-thenable/ https://typescript-eslint.io/rules/await-thenable/ https://typescript-eslint.io/rules/no-for-in-array https://typescript-eslint.io/rules/no-for-in-array https://typescript-eslint.io/rules/no-duplicate-type-constituents https://typescript-eslint.io/rules/no-duplicate-type-constit...
- Too 3y ago
  Is there no incremental lint mode? When developing you need that for instant feedback, same mechanism should work for CI.
arp242 3y ago
One problem is that a change in a.js may trigger a new error in b.js. ESLint could also cache things fairly trivially: hash = hash_file_contents() if previously_seen_hashes.contains(hash) report_previous_results() else run_lint_and_cache_results() end maybe that already exists. But that has the same problem. When you've got enough hardware to throw at it, then "just run it on the full code" is the safest.

msoad 3y ago

I thought it would be obvious that in large codebases you only lint changed files in CI