NYCPHP Meetup

NYPHP.org

[nycphp-talk] Code Reviews

Peter Becker peterbsemail at gmail.com
Mon Jan 4 12:04:23 EST 2010


Ben, David, Tom,

Thanks for the insightful input..  Sorry for the delay in responding, 
but was gonnnnnnnnne over the holidays. 

My take away is that they can be useful if applied in a manner that 
avoids "pissing contests" between specific approaches, driven from above 
with specific objective(s) in mind.  Variations on recurrence sounds 
somewhat variable based on what is being done ie if in Agile environment 
there might be one at end of iteration and/or minimally release.  Unit 
testing is a "given" regardless of whether or not there are code reviews.

We do have a style guide, defined and documented directory structure and 
naming convention.  My objective(s) in the review is to ensure that 
conventions are being maintained (easy to get lax in and nothing that 
would ever show up in testing), and that coders (and testers) stay at 
least familiar with the other coders work (should back filling be required).

And the point to keep it clean and well documented for future ease of 
outsourcing or bringing on new resources is dead on with where I was 
looking - even if there may be some additional time/costs up front, but 
think if done properly can easily pay off down the line.

Thanks again,
Peter

Ben Sgro wrote:
> Hello Peter,
>
> At my current company we do both peer reviews and group code reviews.
> The group code reviews seem to have the best impact, as the peer 
> reviews have naturally ceased to happen.
>
> Our group code reviews happen bi-monthly and during the time (usually 
> an hour) developers working on different
> clients within the company will show high level (run the code, 
> interact w/the UI, etc) and also walk through the source
> code showing all the major components.
>
> The audience is typically programmers, but is open to anyone within 
> the company.
>
> In the past I've also worked in companies where a senior developer 
> will review junior developers major commits. What defines a "major 
> commit" differs within companies, but this was a great way to make 
> sure the new hires are obeying coding standards, not introducing new 
> bugs, and are
> using appropriate libraries ("hey, we already solved that, see abc 
> lib...").
>
> I think the reason that our current peer reviews died was because they 
> are time consuming and they were not manager driven, they were peer 
> driven, so people would either be too busy to schedule something with 
> a peer, or just forget. Plus, since management isn't' involved, 
> besides getting a peer to compliment your work, management wouldn't 
> directly hear of your accomplishments via this channel.
>
> > "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,"
> I don't think this is true - even on a small team I find most 
> developers are not that familiar with the work of their teammates.
>
> - Ben
>
> Peter Becker 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 -
>>
>> ------------------------------------------------------------------------
>>
>> _______________________________________________
>> 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
>



More information about the talk mailing list