[pygr-notify] [pygr commit] r161 - Created wiki page through web user interface.

[codesite-noreply at google.com]

Author: cjlee112
Date: Wed Mar 25 21:46:31 2009
New Revision: 161

Added:
    wiki/CodeReviewGuidelines.wiki

Log:
Created wiki page through web user interface.

Added: wiki/CodeReviewGuidelines.wiki
==============================================================================
--- (empty file)
+++ wiki/CodeReviewGuidelines.wiki	Wed Mar 25 21:46:31 2009
@@ -0,0 +1,24 @@
+#summary Recommendations for how to do Pygr code reviews.
+
+= Keep "code reviewing" and "code rewriting" separate! =
+
+We review existing code by having the reviewer directly edit the code in a  
separate git branch.  When the reviewer edits existing code in this way  
s(he) needs to keep two different impacts separate, by keeping them in two  
separate git branches:
+
+  * "innocuous" revisions: e.g. adding or editing comments or docstrings.   
We will refer to this as "code reviewing" because it leaves the original  
behavior unchanged.
+
+  * significant revisions: anything that might alter how the code  
behaves.  We will refer to this as "code rewriting" because it could change  
the code's correctness and must be analyzed much more carefully.
+
+Once the reviewer is done, the original author(s) of the existing code  
should review these recommended changes and decide what should be merged to  
master.  To make this job of "reviewing the code review" as straightforward  
as possible, it is very important that the innocuous changes (code  
reviewing) be separate from the potentially significant changes (code  
rewriting).  Knowing which changes require "paranoid" analysis (code  
rewriting) vs. which don't will make it easier and faster to review all  
these changes.
+
+Git makes this separation really easy, and makes the potential  
disadvantages / overhead fairly minor.
+
+= Process =
+Say you were performing a code review of the *seqdb* module.  You would  
first
+
+  * create a branch named something like *seqdb_review*, and add  
docstrings and comments, reformat whitespace, add new test cases, move  
classes and functions around, delete unused classes or functions, change  
internal variable names etc.
+
+  * If you see code that you think would benefit from revision, create a  
branch *seqdb_rewrite*, and make your changes there.
+
+  * Always commit each individual change separately (rather than combining  
several distinct changes into one commit), so that it will later be  
possible to easily pick and choose which changes to keep.  If many changes  
are combined in one commit, the only way to separate them is by hand, and  
much of the value of git for managing the revision process is lost.  This  
applies equally both to "code reviewing" and "code rewriting".
+
+  * If code is rewritten, then in effect we have to begin a new cycle of  
code review, to review the new code.  The above rules would apply again;  
i.e. we'd create a review branch in which the existing code's author(s) can  
add comments, tests, etc.  If the author thinks significant changes to the  
rewritten code are desirable, s(he) can either just communicate them to the  
reviewer, or create a new "rewrite" branch in which to make these changes  
and expose that branch for review...
\ No newline at end of file