Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update ESLint warning thresholds, and produce them consistently #5151

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

mcmire
Copy link
Contributor

@mcmire mcmire commented Jan 14, 2025

Explanation

The ESLint warning thresholds file records warnings that were produced the last time that eslint was run, categorizing them by rule. This file should be kept up to date at all times so we can know whether new code changes introduce a regression in the number of warnings or an improvement. Specifically, the lint step will now fail if more warnings are introduced to the codebase.

However, in the past we have observed inconsistencies in the behavior of ESLint across different people's machines as well as in CI. With the addition of the quality gate this means that some people are unable to merge PRs because warnings are produced that are not seen in development, and the warnings may not be reproducible by other developers.

With that said, we recently learned that if dist directories are present from a previous run of yarn build, it could affect the behavior of the TypeScript-specific lint rules and cause the observed inconsistencies. To mitigate this problem, this commit ensures that all dist directories are removed before running eslint. This indeed does change the number of warnings produced, and the thresholds file has been updated to reflect this.

References

Changelog

(N/A)

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@mcmire mcmire marked this pull request as draft January 15, 2025 15:48
@mcmire mcmire force-pushed the update-warning-thresholds branch 2 times, most recently from ea7cfe3 to dc33769 Compare January 15, 2025 17:34
@mcmire mcmire changed the title Update ESLint warning thresholds Update ESLint warning thresholds, and produce them consistently Jan 15, 2025
@mcmire mcmire force-pushed the update-warning-thresholds branch from a1f6074 to 895ba6a Compare January 15, 2025 19:58
The ESLint warning thresholds file records warnings that were produced
the last time that `eslint` was run, categorizing them by rule. This
file should be kept up to date at all times so we can know whether new
code changes introduce a regression in the number of warnings or an
improvement. Specifically, the lint step will now fail if more warnings
are introduced to the codebase.

However, in the past we have observed inconsistencies in the behavior of
ESLint across different people's machines as well as in CI. With the
addition of the quality gate this means that some people are unable to
merge PRs because warnings are produced that are not seen in
development, and the warnings may not be reproducible by other
developers.

With that said, we recently learned that if `dist` directories are
present from a previous run of `yarn build`, it could affect the behavior
of the TypeScript-specific lint rules and cause the observed
inconsistencies. To mitigate this problem, this commit ensures that all
`dist` directories are removed before running `eslint`. This indeed does
change the number of warnings produced, and the thresholds file has been
updated to reflect this.
@mcmire mcmire force-pushed the update-warning-thresholds branch from 895ba6a to b6c8366 Compare January 15, 2025 19:59
@mcmire mcmire marked this pull request as ready for review January 15, 2025 20:04
@mcmire mcmire merged commit f418952 into main Jan 15, 2025
117 checks passed
@mcmire mcmire deleted the update-warning-thresholds branch January 15, 2025 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants