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

Persistent handling of issues #5

Closed
maxnordlund opened this issue Nov 17, 2015 · 10 comments
Closed

Persistent handling of issues #5

maxnordlund opened this issue Nov 17, 2015 · 10 comments

Comments

@maxnordlund
Copy link
Contributor

I really like that this tool is all about presenting, vs enforcing, various code issues. But if I, or people in my team, decides that something is not a problem it would be very nice if credo could remember that.

Maybe something like credo ignore <type|issueno> <source:lineno:columnno> and a suitable file to store it. To actually implement this I would either store the entire offending line, or perhaps a hash of it. Then use this to detect if the line has changed and reenable the check.

There probably needs an corresponding credo clean command to clear out old ignored lines. This should be separate so that running the credo commands doesn't remove ignores while editing.

I would also recommend adding this to version control to share the knowledge and track which & why certain issues have been ignored.

@rrrene
Copy link
Owner

rrrene commented Nov 17, 2015

This is something I also thought about. It would be a non-trivial feature, so I am afraid this is more in the 1.0-territory.

So I absolutely agree with you. It's just that I always criticise it when other projects set the wrong priorities during the very early phases of their development and I don't want to be hypocrite.

Let's keep this issue open to remind me to put it on the roadmap! 👍

@maxnordlund
Copy link
Contributor Author

Yeah, you're totally right, so maybe add a label longterm or something similar.

@rrrene
Copy link
Owner

rrrene commented Feb 10, 2016

@maxnordlund Credo v0.3.0 now includes support for a @lint attribute. These allow you to ignore indiviual checks on a perfunction basis. See CHANGELOG for further details.

What do you think?

@maxnordlund
Copy link
Contributor Author

I love it 😸
This combined with a good commit message should suffice to close this. But I would like a flag to temporarily ignore the @lint attribute. Something one can run periodically to see if the ignores are still valid.

Close this if you like, unless you have a plan for this somehow 😉

@rrrene
Copy link
Owner

rrrene commented Feb 10, 2016

I like this idea of linting the @lint attributes, which could result in a message like

"XYCheck ignored via @lint, but XY does not report any issues here."

Great idea! 👍

@maxnordlund
Copy link
Contributor Author

Yes, yes! Lint all the things. If it will work like that it could probably be on all the time instead of hidden behind a flag. Which would mean your linting ignores will never be grow stale.

Den 10 feb. 2016 12:56 skrev "René Föhring" notifications@github.com:

I like this idea of linting the @lint attributes, which could result in a
message like

"XYCheck ignored via @lint https://github.com/lint, but XY does not
report any issues here."

Great idea! [image: 👍]


Reply to this email directly or view it on GitHub
#5 (comment).

@rrrene
Copy link
Owner

rrrene commented Feb 10, 2016

Yeah, that's what I was thinking: a @lint checker that's just a regular check ... 😱

@chrismcg
Copy link

Yes, this is nice when you finally refactor some bit of code that needed ignored for a while and get to remove the @lint. It's also nice for "lint as you type" editor integration.

@TheFirstAvenger
Copy link
Contributor

@rrrene This was created prior to the credo:disable feature, and I believe that is sufficient to close this issue?

@maxnordlund
Copy link
Contributor Author

Yeah, I think so too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants