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

Allow inline comments to disable particular warnings #160

Open
japhib opened this issue Mar 9, 2023 · 1 comment
Open

Allow inline comments to disable particular warnings #160

japhib opened this issue Mar 9, 2023 · 1 comment

Comments

@japhib
Copy link
Contributor

japhib commented Mar 9, 2023

While it's great to have a .gradient_ignore.exs file where you can specify particular warnings to ignore, it would be awesome if you could do it with inline comments as well.

For instance, when using Credo you can add a comment like # credo:disable-for-next-line Name.Of.Specific.Lint.To.Disable on the line before a warning occurs.

One reason for this is that when the warning-disabling stuff is far away from the code it's affecting, it's more likely to experience drift, becoming out of sync with the code it references. For instance, you might have a warning disabled, and then change the code so the warning no longer occurs, but forget to also update .gradient_ignore.exs to remove that warning-disabling. On the other hand, if the warning-disabling stuff is right next to the code that is being changed, it's easy to remember to update it, or for someone to look at a diff and see that the comment adjacent to the changes might not

Another reason for this is scalability. I'm in the (long :P) process of adopting Gradient for a huge Elixir project at my company that contains thousands of files and hundreds of thousands of lines of code. There are going to be hundreds of warnings we'll want to temporarily disable as we work through inaccurate type hints and other issues. If all these disabled warnings go into one single file, that file will be huge and disorganized. I'd much rather offer our engineers the option of disabling a warning via a comment in the actual file it's affecting.

Hopefully this issue makes sense! I'll be out of office for about a week starting tomorrow, but when I return, I hope to take a stab at an MR for this.

@japhib
Copy link
Contributor Author

japhib commented May 1, 2023

Implemented in this PR: #163

Could I get a review? @erszcz @luk-pau-es

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

No branches or pull requests

1 participant