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

Ability to provide a reason for rule disabling #190

Closed
jBugman opened this issue Jul 26, 2019 · 4 comments · Fixed by #193
Closed

Ability to provide a reason for rule disabling #190

jBugman opened this issue Jul 26, 2019 · 4 comments · Fixed by #193
Assignees

Comments

@jBugman
Copy link

jBugman commented Jul 26, 2019

I would like the ability to provide a reason for rule disabling (on the same line) as an option. Adding anything to the revive: comment breaks it (regex I guess?), so currently I must do it on a separate line.

I propose two things:

  1. Ability to write comments like
    // revive:disable-line:RULE This is intentional
    or even (for cleaner integration with other tools)
    // revive:disable:var-naming TODO: fix it later

  2. Global config flag (disabled by default) that would require to provide the reason (like staticcheck does by default)

@mgechev
Copy link
Owner

mgechev commented Jul 26, 2019

This makes sense. I'll look at it over the weekend. If anyone has bandwidth to take it before that please comment here.

@chavacava
Copy link
Collaborator

Hi, I've proposed something similar last year (see #73)
I'll try to create a PR this weekend.

@chavacava
Copy link
Collaborator

I have a first running version without the config flag.
Suggestions for the name of this flag?

@mgechev
Copy link
Owner

mgechev commented Jul 27, 2019

That was fast!

specifyDisableReason maybe. It's a bit long but expresses well what it's all about. Open to other suggestions!

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 a pull request may close this issue.

3 participants