Group Code Reviews
Overview
In the past, OpenMRS developers conducted real-time group code review sessions. This is now generally discontinued in favor of Asynchronous Code Reviews. See the main Code Review wiki page for a description of the preferred method of reviewing code.
If you would like a group code review, email the developers mailing list to request one.
Triggering a Code Review
The weekly code reviews are driven by the Road Map and all issues that are in Code Review status. Changesets that have been committed by any developer to trunk should get a new code review opened in Crucible.
The issue acts as a formal request for code review by a developer or some other party. Issues should only be marked as "Code Review (Pre-Commit)" once the branch/patch author's code is:
Well commented
Unit tested
Formatted correctly
For more information, see the checkstyle ant target and OpenMRS Conventions .
Process
Each week, code to be reviewed is selected before the meeting.
The meeting moderator shares his or her desktop using WebHuddle, vnc, Vyew, or OpenMRS Connect.
The code review is conducted using the Code Review Checklist.
The code review moderator should have a code compare open:
Check out the latest branch into a project
Check out the most recent trunk for the last time that branch was merged to
Select root folder for both projects and right click --> Compare with --> Each Other
Notes about all that is discussed are written into the notes page for that day.
Tips
Take care not to let the code review turn into a hack-a-thon.
The meeting moderator should make sure that the review stays on task.
The code review should focus on detecting and recording, not fixing bugs.
After Code Review
Any comments or code changes made during the code review should be committed. These changes should be minor only.
The TODO items in the notes should be turned into tickets.
The "parent" ticket's "Code Review Status" should be changed to "Reviewed"
The "parent" ticket should link to each ticket created by the code review
Once all "subtickets" have been completed, the "Code Review Status" should be changed to "Needs Review" again
This process continues until the ticket is marked as "Approved" and then either a core developer or the patch author can commit it to trunk.