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.

Sign up for GitHub

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

Jump to bottom

Add maxWarnings as a configuration option #16804

Closed
1 task
lourd opened this issue Jan 21, 2023 · 6 comments
Closed
1 task

Add maxWarnings as a configuration option #16804

lourd opened this issue Jan 21, 2023 · 6 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint

Comments

@lourd
Copy link

lourd commented Jan 21, 2023

ESLint version

8.32.0

What problem do you want to solve?

Resurrecting #13391:

I want to make having a max warnings config value of 0 opt-out instead of opt-in.

My team has a monorepo managed with Nx. It's setup where we share most configuration for each target (library or application) in a root .eslintrc.json, which each target then extends in its own .eslintrc.json file.

One key configuration that can't be shared is the max warnings. When making a new library or application, the developer has to remember to add that to the additional configuration file (in Nx's case, the configuration for the eslint executor in the project.json).

This often gets forgotten, causing linter warnings build up and making it difficult to get back into good shape.

What do you think is the correct solution?

I'd like eslint to have the ability to set maxWarnings as a property in a .eslintrc.json file, not just through the command line.

Participation

  • I am willing to submit a pull request for this change.

Additional comments

No response
@lourd lourd added core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint triage An ESLint team member will look at this issue soon labels Jan 21, 2023
@nzakas
Copy link
Member

nzakas commented Jan 31, 2023

We can consider this again, but keep in mind that allowing maxWarnings to be set in a configuration file also means that any configuration file you import from anywhere could set it without you being aware.

@nzakas nzakas removed the triage An ESLint team member will look at this issue soon label Jan 31, 2023
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 8, 2023
@KKoukiou
Drop webpack in favor of esbuild
a21a0f7 
TODO:
[ ] cockpit-po-plugin
[ ] fail on warnings eslint (Open eslint issue eslint/eslint#16804)
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 8, 2023
@KKoukiou
Drop webpack in favor of esbuild
d128e43 
TODO:
[ ] cockpit-po-plugin
[ ] fail on warnings eslint (Open eslint issue eslint/eslint#16804)
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 9, 2023
@KKoukiou
Drop webpack in favor of esbuild
c1a329f 
TODO:
[ ] cockpit-po-plugin
[ ] fail on warnings eslint (Open eslint issue eslint/eslint#16804)
[ ] string replace loader does not work
[ ] css from Patternfly is not being extracted
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 9, 2023
@KKoukiou
Drop webpack in favor of esbuild
857a5df 
TODO:
[ ] cockpit-po-plugin
[ ] fail on warnings eslint (Open eslint issue eslint/eslint#16804)
[ ] string replace loader does not work
[ ] css from Patternfly is not being extracted [1]

[1] evanw/esbuild#2905
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 9, 2023
@KKoukiou
Drop webpack in favor of esbuild
fb0c998 
TODO:
[ ] cockpit-po-plugin
[ ] fail on warnings eslint [0]
[ ] string replace loader does not work
[ ] css from Patternfly is not being extracted [1]

[0] eslint/eslint#16804
[1] evanw/esbuild#2905
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 12, 2023
@KKoukiou
Drop webpack in favor of esbuild
93b6cdb 
TODO:
[ ] cockpit-po-plugin
[ ] fail on warnings eslint [0]
[ ] string replace loader needs to replace fake pths
[x] css from Patternfly is not being extracted [1]

[0] eslint/eslint#16804
[1] evanw/esbuild#2905
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 14, 2023
@KKoukiou
maint: Drop webpack in favor of esbuild
d43945d 
For eslint integration there are two existing plugins [1], [2]
but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
[ ] cockpit-po-plugin
[ ] cockpit-rsync-plugin
[ ] stylelint intergration
[ ] fail on warnings eslint [0]

[0] eslint/eslint#16804
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 14, 2023
@KKoukiou
maint: Drop webpack in favor of esbuild
cd7f46d 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
[ ] Bundle sizes are much bigger for eslint as files are not compressed
[ ] fail on warnings eslint [0]

[0] eslint/eslint#16804
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 14, 2023
@KKoukiou
maint: Drop webpack in favor of esbuild
c1ddbb8 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
[ ] Bundle sizes are much bigger for esbuild as files are not
compressed, consider if this is needed
[ ] fail on warnings eslint [0]

[0] eslint/eslint#16804
KKoukiou added a commit to KKoukiou/cockpit-podman that referenced this issue Feb 14, 2023
@KKoukiou
maint: Drop webpack in favor of esbuild
0eda1f3 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
[ ] Bundle sizes are much bigger for esbuild as files are not
compressed, consider if this is needed
[ ] fail on warnings eslint [0]

[0] eslint/eslint#16804
martinpitt pushed a commit to martinpitt/cockpit-podman that referenced this issue Feb 17, 2023
@KKoukiou @martinpitt
maint: Drop webpack in favor of esbuild
fa15aad 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
[ ] Bundle sizes are much bigger for esbuild as files are not
compressed, consider if this is needed
[ ] fail on warnings eslint [0]

[0] eslint/eslint#16804
martinpitt pushed a commit to martinpitt/cockpit-podman that referenced this issue Feb 19, 2023
@KKoukiou @martinpitt
maint: Drop webpack in favor of esbuild
a55bd6f 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
[ ] Bundle sizes are much bigger for esbuild as files are not
compressed, consider if this is needed
[ ] fail on warnings eslint [0]

[0] eslint/eslint#16804
martinpitt pushed a commit to martinpitt/cockpit-podman that referenced this issue Feb 19, 2023
@KKoukiou @martinpitt
maint: Drop webpack in favor of esbuild
c580e08 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
- [ ] Bundle sizes are much bigger: evanw/esbuild#2933
- [ ] fail on eslint warnings eslint: eslint/eslint#16804
- [ ] sass plugin pulls in older/duplicate esbuild compiler: glromeo/esbuild-sass-plugin#122
@martinpitt martinpitt mentioned this issue Feb 19, 2023
Merged
13 tasks
martinpitt pushed a commit to martinpitt/cockpit-podman that referenced this issue Feb 20, 2023
@KKoukiou @martinpitt
maint: Drop webpack in favor of esbuild
926d27d 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
- [ ] Bundle sizes are much bigger: evanw/esbuild#2933
- [ ] fail on eslint warnings eslint: eslint/eslint#16804
- [ ] sass plugin pulls in older/duplicate esbuild compiler: glromeo/esbuild-sass-plugin#122
martinpitt pushed a commit to martinpitt/cockpit-podman that referenced this issue Feb 20, 2023
@KKoukiou @martinpitt
maint: Drop webpack in favor of esbuild
3dedc77 
Esbuild performance:
$ time npm run build

> build
> node build.js

real	0m7.747s
user	0m12.328s
sys	0m0.508s

Webpack performance:
[kkoukiou@sequioa cockpit-podman]$ time npm run build

> build
> webpack

Webpack compiled successfully

real	0m26.832s
user	0m47.778s
sys	0m1.640s

For eslint integration there are two existing plugins [1], [2]

but none can be used because of unsatisfied peer dependency version of
esbuild. Let's just use our own eslint plugin for now.

[1] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
- [ ] Bundle sizes are much bigger: evanw/esbuild#2933
- [ ] fail on eslint warnings eslint: eslint/eslint#16804
- [ ] sass plugin pulls in older/duplicate esbuild compiler: glromeo/esbuild-sass-plugin#122
martinpitt added a commit to martinpitt/cockpit-podman that referenced this issue Feb 20, 2023
@martinpitt
esbuild-{es,style}lint-plugin.js: Fix error behaviour
1cf6b07 
Properly report errors. Unfortunately errors cause eslint watch mode to
abort the watch loop, so add a hack to reduce them to warnings in watch
mode. Warnings don't fail a normal "npm run build" [1], so we have to
make this dynamic for now.

[1] eslint/eslint#16804
martinpitt added a commit to martinpitt/cockpit-podman that referenced this issue Feb 20, 2023
@martinpitt
esbuild-{es,style}lint-plugin.js: Fix error behaviour
0619949 
Properly report errors. Unfortunately errors cause eslint watch mode to
abort the watch loop, so add a hack to reduce them to warnings in watch
mode. Warnings don't fail a normal "npm run build" [1], so we have to
make this dynamic for now.

[1] eslint/eslint#16804
@github-actions
Copy link

github-actions bot commented Mar 2, 2023

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

@github-actions github-actions bot added the Stale label Mar 2, 2023
@Rec0iL99
Copy link
Member

Rec0iL99 commented Mar 3, 2023

@eslint/eslint-team folks your feedback here would be appreciated.

maxWarnings in .eslintrc.* was originally proposed in #13391, but was rejected then because of eslint/rfcs#9.

@lourd are you interested in implementing this if approved by the team? The core team is currently focused on implementing a new config system. You can track it's progress here #13481 and rfc.

@mdjermanovic
Copy link
Member

I'm not sure if maxWarnings: number, as the equivalent of the CLI option --max-warnings, makes sense in configuration files because it's not max per file but max per run, so it's conceptually related to a specific run. For example, in this repository we have one configuration file but two commands that run eslint on different files (eslint . --ignore-pattern "docs/**" and eslint docs). If we'd use --max-warnings, it would have different values ​​in different commands rather than a single value in the configuration file.

@github-actions github-actions bot removed the Stale label Mar 3, 2023
@nzakas
Copy link
Member

nzakas commented Mar 8, 2023

@mdjermanovic yeah, that's my feeling as well. I don't believe it belongs in the config file, so closing.

@nzakas nzakas closed this as not planned Won't fix, can't repro, duplicate, stale Mar 8, 2023
@lourd
Copy link
Author

lourd commented Mar 8, 2023

Those points all make sense. Thank you for the thoughtful consideration!

@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Sep 5, 2023
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint
Projects
Triage
Archived in project
Milestone
No milestone
Development

No branches or pull requests
4 participants
@nzakas @lourd @mdjermanovic @Rec0iL99