Add TODO Comment Checker and associated unit tests #134
Conversation
|
Added a FIXME check as requested by HairyFotr |
| </example-configuration> | ||
| </check> | ||
|
|
||
| <check id="fixme.comment"> |
There was a problem hiding this comment.
This shouldn't be in twice
|
@HairyFotr It seems like overkill to have two classes. Please note that you can have the same class twice in the configuration, but with different parameters. Could you confirm that this would be acceptable? |
| /** | ||
| * Checks for line comment style FIXMEs | ||
| */ | ||
| class FixmeCommentChecker extends FileChecker { |
There was a problem hiding this comment.
This seems to be exactly the same code as the FixmeCommentChecker - you could use the same abstract class for them both. Also, I'm not sure that we need both classes anyway - see general comments.
|
Cool, I didn't know the same class can be used multiple times. Yes, one class is enough, then. |
|
So I've created an abstract class for Todo and Fixme to extend. I wasn't sure about using just one class. How would the errorKey and id work? |
|
Sorry it took so long to get back to this. Could you do a rebase and a squash for this please? |
Change-Id: I0e9059fe00491c03bb8efe0590cc51bb25e3e1df
|
Rebased and squashed as requested. It should be good. |
Added codecov
|
I'm going to accept this PR, but I think we can do better for configuration - the regex seems very complicated and hard to get right. I'll change it to have one single string where you can specify TODO or FIXME etc, similarly to http://checkstyle.sourceforge.net/apidocs/com/puppycrawl/tools/checkstyle/checks/TodoCommentCheck.html |
Add TODO Comment Checker and associated unit tests
Not sure what happened to the old pull request (#133). This one covers off a couple of suggestions by HairyFotr