This post is mainly meant to aggregate links to the topics that I talked about.
I covered several tools. All of these should be available to the developers as build targets and run in your continuous build. Lint and your unit tests should be run as part of your submission process.
- lint - The bare minimum, it just detects syntax errors in your scripts. Code that doesn't pass the lint test won't pass any other tests or manual QA.
- PHPUnit - Standard unit testing framework. There is plenty of information about it elsewhere.
- PHP Code Sniffer - Detects code smells that should be fixed. Many bad programming practices can be written as "sniffs" along with most rules from your smile guide.
- PHP Mess Detector - Statically analyzes your code for possible bugs or coding practices that tends to hide bugs.
- PHP Copy Paste Detector - Scans your code to find large similar blocks which can be factored out to a common method.
- PHP Dead Code Detector - Scans your code to find code that can not be reached. For example, code after a return statement.
- Code coverage - Adding the xdebug extension to your system allows PHPUnit to calculate how much of your code is run by unit tests.
I talked about two different code review packages:
- Review Board - You install it on your server and host it yourself.
- Codereview.appspot - Use Google's code review tool.
And I talked about three different ways of doing reviews:
- Pre-review - Code doesn't get submitted until a peer reviews it. Keeps bad stuff out of your code base.
- Post-review - Code gets submitted, then gets peer reviewed. Comments made about the code may never get resolved, but code reviews don't slow down getting code into production.
- Public shaming - Put the code up on a projector and discuss as a team. Great way to destroy programmer morale.
I mentioned a few points about what to look for in a code review:
- anything the tools couldn't catch
- logic errors (like ifs that don't make sense)
- loops with off-by-one errors
- performance problems (SQL in a loop)
- things to refactor (large methods)
- or things they missed
- Style problems (not really wrong, but you know, wrong)
- Typos (variable names, documentation)
- Tests that don't have assertions
- Methods without tests
There's two ways to choose a style guide: