| One process I've used in the past which I thought was quite succesful was: 1) Entire team must do code review. Even if it's just a member clicking "Decline, code review done already by LandR" 2) We used 3 levels. * Finished, Looks Good - This is good, you can check it in to our dev branch. * Finished with comments - This is good, but I have a few comments, if you make the changes in teh comments its good to check in to our dev branch. * Finished, Needs Work - This code is bad. There is a big issue here and you need to look at it / rewrite it. The new code will need a new code review. DO NOT check in what you have. All check ins were gated and no check ins allowed unless everyone has either finished the review with Finished Looks Good or finished with comments. If there was a dispute, e.g. 4 Finished with Comments and 1 Finished Needs Work. Then you would need to have a conversion with the Finished needs Work person (or if they are right, ignore the results from the others). This process kept code quite tight. What you found was though the more junior members were almost always Finished Looks Good,because they couldn't recognise issues. The problem was though, afterwards when they had done their "finished looks good", they didn't look at the code review again.. If you can get the more junior members involved in everyones code review and get them to look at comments on >other peoples code< from the more senior members it can be a good learning opportunity. |