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

Enable nolintlint linter #469

Open
jeanduplessis opened this issue Feb 12, 2025 · 1 comment
Open

Enable nolintlint linter #469

jeanduplessis opened this issue Feb 12, 2025 · 1 comment
Labels
enhancement New feature or request

Comments

@jeanduplessis
Copy link
Collaborator

There currently exists a pattern of //nolint directives disabling linters without saying why.
We should enable the nolintlint linter, that'll require explanatory comments for //nolint directives.
This helps the reader understand why a //nolint directive was appropriate.
As a bonus, it'll also automatically remove //nolint directives that don't do anything anymore.

@jeanduplessis jeanduplessis added the enhancement New feature or request label Feb 12, 2025
@negz
Copy link
Member

negz commented Feb 13, 2025

I think it'd be a good idea to take the linter configuration from c/c and replicate it here.

A while back we changed c/c to "opt-in" for all linters rather than opt out. The migration is a bit toilsome due to all the newly enabled linters but I feel it's a good thing overall. While working in this repo for the last few days I've certainly seen a lot of little cases where I've thought "I wish a linter caught that".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants