Changes between Version 5 and Version 6 of PatchReviewProcess

Timestamp:

2010-03-19T05:25:23Z (14 years ago)

Author:

zooko

Comment:

edit keyword, add link to jml's essay

PatchReviewProcess

@@ -5 +5 @@
 We want more patches to be contributed to Tahoe-LAFS.  Getting  
 feedback on patches encourages contributors.  Patches languishing in  
-the "waiting to be reviewed" queue discourages them.  (By the way,  
+the "waiting to be reviewed" state discourages them.  (By the way,  
 something else that encourages them is users saying "Thank you.".)
 
@@ -17 +17 @@
 = How to review patches =
 
- 1. Go to http://allmydata.org .  Click on "[http://allmydata.org/trac/tahoe/wiki/ViewTickets View Tickets]".  Click on "[http://allmydata.org/trac/tahoe/query?order=priority&keywords=~review-needed review-needed]".
+ 1. Go to http://tahoe-lafs.org .  Click on "[http://tahoe-lafs.org/trac/tahoe/wiki/ViewTickets View Tickets]".  Click on "[http://tahoe-lafs.org/trac/tahoe/query?order=priority&keywords=~review-needed review-needed]".
  2. You can read everything without registering, but to add comments or change tickets you have to be logged in.  Registering is quick and easy -- click the "Register" link at the top right of the page.
  3. Read tickets until you find one that you can review.
  4. (optional) Click "accept".  This marks you as the person reviewing this patch.  If you don't want to commit to this then you can skip this step.
- 5. Read the patch until you understand all of the docs, tests, code and comments in it.  You can use the "Browse source" button at the top of the page to read the current versions of the files that the patch changes.
+ 5. Read the patch until you understand the docs, tests, code and comments in it.  You can use the "Browse source" button at the top of the page to read the current versions of the files that the patch changes.
   a. If you can't understand the patch after spending some time on it, then say so in a comment on the ticket!  This might be taken as a reason to add documentation or comments or to refactor the code.  On the other hand, it might just be that you don't have enough context to understand the code.  That's okay too.
-  b. If you find errors or omissions in the docs, tests, code or comments then write that down in the ticket, remove the "review" keyword from the keywords, and assign the ticket to someone other than yourself.  (That would be the original author of the patch, or someone who seems likely to fix the patch, or if you can't think of anyone better then assign it to me.)
-  c. If you understand the patch and find no errors or omissions then remove the keyword "review", add the keyword "reviewed" and assign it to someone with repository write access (currently 'zooko', 'warner' and 'davidsarah'). We'll commit it to trunk.
+  b. If you find errors or omissions in the docs, tests, code or comments then write that down in the ticket, remove the "review-needed" keyword from the keywords, and assign the ticket to someone other than yourself.  (Assign it to the original author of the patch, or someone who seems likely to fix the patch, or if you can't think of anyone better then assign it to Zooko.)
+  c. If you understand the patch and find no errors or omissions then remove the keyword "review-needed", add the keyword "reviewed" and assign it to someone with repository write access (currently 'zooko', 'warner' and 'davidsarah'). We'll commit it to trunk.
   d. Feel good about yourself.  Thank you for helping with our little project attempting to improve the world!
+
+= Advanced =
+
+Once you decide that you like reviewing tickets and you get into the habit of doing it frequently, then read this essay by Jonathan Lange on how to do it well: [http://mumak.net/stuff/your-code-sucks.html Your Code Sucks and I Hate You: The Social Dynamics of Code Reviews].