Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1341674_codecheck into lp:widelands

2014-07-25 Thread GunChleoc
I think this is done now - except for 1 empty TODO comment that I left around. 
Maybe someone else will know why there's a TODO there:

src/io/filesystem/zip_filesystem.cc:563: Use TODO(username): msg. Do not 
add empty TODO comments.

I have also made the feedback individual to each case, so the user will know 
why the rule barfed specifically.
-- 
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.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1341674_codecheck into lp:widelands

2014-07-25 Thread GunChleoc
Review: Resubmit

Trunk is merged :)
-- 
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.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp


Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/bug-1341674_codecheck into lp:widelands

2014-07-23 Thread SirVer
Review: Needs Fixing



Diff comments:

 === added file 'cmake/codecheck/rules/format_TODO_comments'
 --- cmake/codecheck/rules/format_TODO_comments1970-01-01 00:00:00 
 +
 +++ cmake/codecheck/rules/format_TODO_comments2014-07-23 14:52:16 
 +
 @@ -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.

___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help   : https://help.launchpad.net/ListHelp