Difference between revisions of "Dev Reviewing Guidelines"

From Dreamwidth Notes
Jump to: navigation, search
(reviewing guidelines first revision)
 
(mention BML)
Line 9: Line 9:
 
* Check for compliance with the DW style guidelines, as outlined on the [[Programming Guideline Checklist]].   
 
* Check for compliance with the DW style guidelines, as outlined on the [[Programming Guideline Checklist]].   
  
* Check for syntax errors if you're familiar enough with the language being used to spot them.  Most DW pages are coded primarily in HTML and Perl, but sometimes you'll run into JavaScript, CSS, or the particular markup used by the S2 system.
+
* Check for syntax errors if you're familiar enough with the language being used to spot them.  Most DW pages are coded primarily in HTML/BML and Perl, but sometimes you'll run into JavaScript, CSS, or the particular markup used by the S2 system.
  
 
* Apply the patch in your development environment and check to see that it performs as advertised.  See [[Dev Patches#Applying Patches|how to apply a patch]] if this is new to you.  If the patch won't apply without errors, that's usually an indication that something else has changed in the relevant code since the patch was written, and you can either track down the changes yourself or ask the author to regenerate the patch.
 
* Apply the patch in your development environment and check to see that it performs as advertised.  See [[Dev Patches#Applying Patches|how to apply a patch]] if this is new to you.  If the patch won't apply without errors, that's usually an indication that something else has changed in the relevant code since the patch was written, and you can either track down the changes yourself or ask the author to regenerate the patch.

Revision as of 19:46, 5 July 2009

Getting Started

Before you start, you should be familiar with Bugzilla, which is where contributed code changes are submitted for review. You should ideally also have a working environment on a development system where you can test patches. See Dev Getting Started for more info.

When you're ready, this link will take you directly to the list of review requests in Bugzilla: "review?"

Basics of Patch Review

  • Check for syntax errors if you're familiar enough with the language being used to spot them. Most DW pages are coded primarily in HTML/BML and Perl, but sometimes you'll run into JavaScript, CSS, or the particular markup used by the S2 system.
  • Apply the patch in your development environment and check to see that it performs as advertised. See how to apply a patch if this is new to you. If the patch won't apply without errors, that's usually an indication that something else has changed in the relevant code since the patch was written, and you can either track down the changes yourself or ask the author to regenerate the patch.

Providing Feedback

If the code you've reviewed compiles, works, and conforms to the style guidelines, you should give it your seal of approval.

The way to do this is to click on the "Details" link next to the patch listing on the bug page, and in the popup menu next to the review heading, change the '?' to a '+'. Optionally, you can leave a comment telling the author what great work they've done.

On the other hand, if you've found a problem with the code, you should change the '?' to a '-' and provide the author details of whatever needs to be fixed.

Once a patch has been given the thumbs up by a reviewer, it should be committed by one of the developers with commit privileges, as outlined in the Dev Committing Guidelines. Make sure the commit flag on the patch is set to '?' so that it shows up in the queue of commit requests.