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 sniff for FIXME comments as errors to TodoSniff. #8

Closed
wants to merge 1 commit into from

Conversation

illusori
Copy link
Contributor

Patch to add FIXME comments to the TODO comments sniff: FIXME comments produce an error rather than a warning, and have FixmeCommentFound or FixmeTaskFound message class.

Updated tests included (and test-suite passes successfully).

@gsherwood
Copy link
Member

I think the concept of a FIXME sniff is good, but I don't think it belongs in the TODO sniff. It changes the behaviour of this sniff so it no longer does what the documentation says it does. It also uses errors instead of warnings, which makes the sniff stronger than it currently it.

Do you want to just create a new sniff for this, and unit tests, and I'll merge that in? I know it seems like a copy/paste job, but I think it then keeps the intention of the sniffs clear.

@illusori
Copy link
Contributor Author

illusori commented Jan 9, 2012

Good points, I was half wondering myself if I should have done that.

I'll close this pull request and submit another once I'm done.

@illusori illusori closed this Jan 9, 2012
rhowardiv referenced this pull request in rhowardiv/PHP_CodeSniffer Feb 15, 2013
Remove line breaks from error/warning messages
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.

2 participants