Hi Peter, On Tue, Dec 29, 2009 at 2:30 PM, Peter Becker <peterbsem...@gmail.com> wrote: > Looking to get some views (and best practices) on code reviews. I used to > work at IBM on their early version of Websphere (as UI designer, not coder) > where our group had code reviews on a regular basis. I'm now managing a > small dev team working on a new web site using Zend PHP/MySql and am curious > to know what current practices are for reviews in this sort of environment. > On the one hand it seems like it'd be somewhat redundant since we are a > small team and the left hand does know what the right hand is doing, but on > the other, I could see it being very useful to ensure that everyone is on > the straight and narrow to everyone's benefit (particularly as the team > grows). Any good references as a starting point for approaches? Appreciate > the direction - > >
Let me just start out by saying that I'm not a huge fan of the general "process" of code reviews as I know them: 1. Gathering a bunch of folks into a room to talk about a piece of code is a time-waster IMO - Unless they are specifically working on that portion of the project they are out of context from the get-go 2. You find something wrong, then what? And what if it is not "really" wrong (Jones uses a selection sort but sould have used an insertion sort, and no one told him beforehand). Do we hold up the project until the engs figure out what is best, even though the code "works"? Good luck explaining that to mgmt. And politically, it can make Jones look bad unnecessarily, yet, as the new guy on the team, he might not be familiar with the standards, practices, etc. of your group. I see the typical code-review process as reactive rather than proactive. Your time is better spent investing in a build process that provides code coverage, PMD, checkstyle/codesniffer reporting and of course insisting that developers need to write unit tests. All of this will give you and mgmt a better sense of where the code stands in terms of quality and is all reusable. In addition, daily, automated performance testing will tell you if you're about to release something that won't stand up to your traffic. With that said, I do believe in code reviews for two scenarios: 1. Senior members should review commits of junior members. We do this as a one-on-one mentoring scenario. 2. Something that is broken in production and needs to be fixed right now. When things are broken, people tend to get worked-up. Having someone look over what you're about to check in at that point is a good idea IMO. Thanks, Tom http://www.liphp.org > _______________________________________________ > New York PHP Users Group Community Talk Mailing List > http://lists.nyphp.org/mailman/listinfo/talk > > http://www.nyphp.org/Show-Participation > _______________________________________________ New York PHP Users Group Community Talk Mailing List http://lists.nyphp.org/mailman/listinfo/talk http://www.nyphp.org/Show-Participation
