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

Add checker for TODO comments without detail/assignee #1169

Merged
merged 7 commits into from Jan 26, 2022

Conversation

liamg
Copy link
Contributor

@liamg liamg commented Dec 23, 2021

Resolves #434

cristaloleg
cristaloleg previously approved these changes Dec 28, 2021
Copy link
Member

@cristaloleg cristaloleg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

@quasilyte
Copy link
Member

I think it should be marked with an "opinionated" tag, just in case.
It's not always sensible to require this rule to be followed.
We can merge PR and add this tag separately though, as it's marked with experimental anyway.

@liamg
Copy link
Contributor Author

liamg commented Jan 25, 2022

@quasilyte Thanks, I've addressed your comment. Sorry for the delay. It looks like the cg.List[...].Text fields include the comment initialisers (// and /*) so I've adjusted the regex to take this into account and everything seems to work.

@quasilyte
Copy link
Member

I have a few more suggestions, but I'm OK with merging it as is, so they can be addressed later.
One of my questions is: this is probably more style-related than correctness-related. So the "diagnostic" tag is quite misleading.

@liamg
Copy link
Contributor Author

liamg commented Jan 26, 2022

Cool, I changed it to style and added opinionated too.

@quasilyte quasilyte merged commit 352ed07 into go-critic:master Jan 26, 2022
@liamg liamg deleted the liamg-add-todo-linter branch January 26, 2022 16:02
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 this pull request may close these issues.

lint: TODO/FIX(ME)/BUG/XXX comments without addressing a person
3 participants