[nycphp-talk] Code Reviews

Tom Melendez tom at
Tue Jan 5 12:01:18 EST 2010

Hi Folks,

> 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).

Just in case it wasn't clear in my earlier response and for those who
aren't aware - you can build this stuff into codesniffer.  We do this
kind of thing for everything from tabs and curly brace placement to
variable name case to input validation functions and the precise code
we want used to see if a particular type of object is valid.  You can
then generate a checkstyle reports and you now have a metric to
monitor, rather than a bunch of people in a room looking at a screen
saying "that's not how we instantiate those objects" or whatever.  You
now have something (reusable) there to actually enforce it.

And if you're not already, you can then spend that time on
architecture/design sessions.  The bigger flaw to fix is when it is
wrong inherently by design.  Design sessions and design reviews are
when you want everyone around as that can't (easily) be represented in
codesniffer analysis.

Finally, as your process becomes more refined you can get into code
generation, although ideally, through your design/review sessions
you've abstracted so much that you really don't need to write much new
code anymore to add new features.  And by then, it will be time to
rewrite the application again. :-)

Not trying to be argumentative, just want to make sure you don't end
up with bunch of people in a room for 3 hours and the end result
being, "Yes, Jones will change all of his tabs to spaces".  Those
meetings suck for everyone involved.



More information about the talk mailing list