# HG changeset patch # User Frederic Bastien # Date 1285261026 14400 # Node ID 14444845989a2a63d37ff8aff40646d2af79912b # Parent 808e38dce8d6052af21b52a6f1d67a3014140e4d add stuff to code review following the first meeting. diff -r 808e38dce8d6 -r 14444845989a doc/v2_planning/code_review.txt --- a/doc/v2_planning/code_review.txt Thu Sep 23 12:18:39 2010 -0400 +++ b/doc/v2_planning/code_review.txt Thu Sep 23 12:57:06 2010 -0400 @@ -12,18 +12,19 @@ - make a list of point to compare tools - review interresting projects -- make a decission +- make a politic of review(who,what,what,how) +- make a decission on projects Some system that we should check: --------------------------------- -- `rietveld ` Made by Guido van Rossum -- `Gerrit ` -- `track PeerReviewPlugin ` Could be integrated with the current ticket system? +- `rietveld ` Made by Guido van Rossum, seam basic and svn only +- `Gerrit `, git only +- *`Review Board `_ +- *`Code Striker `, hg added? David told in May 2009 it can do it easily. +- *`Code Review plugins in Redmine ` +- `track PeerReviewPlugin ` Could be integrated with the current ticket system?, not maintained, review code in general, not commit. - `feature request at assembla ` -- `Review Board `_ -- `reviewboard ` -- `Code Striker ` - `JCR ` What we could want from our code review @@ -31,7 +32,13 @@ - integrate with our ticket system? - Should we keep our current ticket system? -- work with mercurial +- work with mercurial, git? +- check each commit of theano/pylearn +- check experimental repository code when asked +- how show diff? patch? syntax highlight as vimdiff? +- If we commit something that is disabled by default and not fully working, we can say it in the commit message to have a faster review(only check that by default it is disabled). Then we should say in the commit message when it is ready for a full review. +- Review should be done by everybody. +- Who choose the reviewer(random, commiter)? pool of reviewers? pool level 1,2,3 where 1 is everybody with commit right. pool for specific topic(gpu, ML algo, ...)? Doc on code review ------------------ @@ -40,11 +47,35 @@ - http://ostatic.com/blog/open-source-code-review-tools - http://en.wikipedia.org/wiki/Code_review -Alternative to code review --------------------------- +Type of code review +------------------- +- Formal review - Many person review together each line of the program. - Over-the-shoulder – One developer looks over the author's shoulder as the latter walks through the code. - Email pass-around – Source code management system emails code to reviewers automatically after checkin is made. - Pair Programming – Two authors develop code together at the same workstation, such is common in Extreme Programming. - Tool-assisted code review – Authors and reviewers use specialized tools designed for peer code review. -- Test-Driven development +- Alternative: Test-Driven development +- Automatic review: use tool as pylint, pyflakes, pychecker. Don't check everything. + +We seam to do Over-the-shoulder, email and variant of pair programming from time to time. Some people read rapidly the commit of Theano and Pylearn. + +Reason for the code review +-------------------------- + +- We want at least 2 people to read all code. That mean we need a reviewer +- This help to find better solution to problem +- This help to train people on our tools ans framework. + +Check list for review +--------------------- + +- Is their tests and do they test all case? +- Is their documentation in the file? + - Do this need doc in the html doc? +- Is the addition well integrated into our framework +- Is the code well placed in the right files and right place in them? +- Try to don't duplicate code +- Is the code clear/comprehensible +- Are the comment describing what is being done? +- Answer question by de commiter, this can also serve to train people