widelands-dev team mailing list archive

[SirVer <SirVer@xxxxxx>]

Review: Needs Fixing



Diff comments:

> === added file 'cmake/codecheck/rules/format_TODO_comments'
> --- cmake/codecheck/rules/format_TODO_comments	1970-01-01 00:00:00 +0000
> +++ cmake/codecheck/rules/format_TODO_comments	2014-07-23 14:52:16 +0000
> @@ -0,0 +1,21 @@
> +#!/usr/bin/python
> +
> +error_msg ="Please use the format \"TODO(<username>): ...\" for your TODO comments, and don't put them in the doygen comments"

Tighten this sentence up a bit. It might be read a lot of time by developers, so it should be short. It also has a typo doygen -> doxygen. Suggestion.:

Use "TODO(username): <msg>. Do not put TODOs in Doxygen comments."

> +
> +regexp = r"""(FIXME|(\s|/|[*])BUG|TODO(?![(])|\Wtodo(?![(])|[*]\s*TODO|///\s*TODO)"""

again, I think a python, line by line version would be simpler to read and execute faster.

> +
> +forbidden = [
> +    "// FIXME this is a todo comment",
> +    "// BUG this is a todo comment",
> +    "// TODO this is a todo comment",
> +    "// TODO: This is a todo comment",
> +    "* TODO: This is a todo comment",
> +    "/// TODO: This is a todo comment",
> +    "\TODO: This is a todo comment",
> +    "\\\todo: This is a todo comment"
> +]
> +
> +allowed = [
> +    "// TODO(<username>) this is a todo comment",

only allow one of these styles. The second one is slightly easier to grep for.

> +    "// TODO(<username>): This is a todo comment"
> +]
> 


-- 
https://code.launchpad.net/~widelands-dev/widelands/bug-1341674_codecheck/+merge/227936
Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/bug-1341674_codecheck.