This post originated from an RSS feed registered with Agile Buzz
by Jared Richardson.
Original Post: A Basic Process Doc
Feed Title: Jared's Weblog
Feed URL: http://www.jaredrichardson.net/blog/index.rss
Feed Description: Jared's weblog.
The web site was created after the launch of the book "Ship It!" and discusses issues from Continuous Integration to web hosting providers.
I was recently asked to create a very simple process document for a very small, very new company. The developers are scattered over several time zones, but we still want to try to grow into a team. The team has very tight timelines and is under a lot of pressure to deliver product (but then, isn't everyone?), so sending everyone to read a book or take a class isn't an option.
This is what I came up with up... I'm not even sure I'd call it a process. It's just a loose collection of a few practices. The team immediately pushed back against the code review clause, so we removed it. I've included it here because I'm curious what everyone else thinks.
If you've got any feedback or stories about any particular points, send them on and I'll gather and repost them.
1) Get Code Reviews
No code should ever be checked in without a code review first.
What's a code review? In person it's two people quickly scanning through the code with the author explaining the intent to the reviewer. It can be more than one reviewer, but one's usually enough. The author takes the reviewer's feedback and considers it. Something has to be Very Wrong for a veto to be invoked, but there are always suggestions related to style, best practices, format, etc. Feel free to offer suggestions but don't offended if the author chooses not to adopt every change wholesale.
How can we have distributed code reviews? I've had success with code diffs being mailed around, along with a short paragraph describing what I intended to code. We can then follow up with a Skype or IM session.
The idea is to have us both look over each line of the code with the author explaining the code to the reviewer. If we're talking over Skype and looking at the same doc, it might be harder for me to point at a code block, but I think it can still work.
Quite often the reviewer will see something that needs to change, but just as often, we'll find a problem in our own code. It's a variation on the Pragmatic Programmer's "Talk to the Duck" idea. In explaining our code to someone else, we often find previously missed bugs.
Why get the review ~before~ the code is checked in? I've found that most people don't want to clean up their code if it's already been checked in. We'll fix errors, but we don't want to adjust text formatting or variable names if the code has been checked in. I'd rather do these slight refactorings that enhance readability or improve the code every time.
So before you push any code, find another team member and get a review.
btw, always ask someone if it's a good time for a review before launching in! They might be in the middle of some deep debugging or coding and you don't want to blow that!
2) Write Meaningful Messages
When you check in code you have to type a message. In this block include:
a) your reviewer
b) the intent of the code
Never leave the message blank... give use enough to understand your intent when you wrote the code. Quite often the code and the developer's intended purpose diverge, making it very difficult for someone else to come in after you and work on the code. I want to know what you meant to write as well as what you wrote.
3) Write automated tests
There are two kinds of tests to focus on. The first is unit tests and the second is integration tests.
With Unit tests, craft your unit tests to a single class. Don't depend on data on disk or in the database. Make sure each class does what you think it does. :) I know I've been very surprised at some of code has done when I actually tested it. It's amazing how much better this will make your code.
Quite often code is "tested through use." That means any methods or routines not invoked are untested. It also means that any use cases not exercised by the client code are untested. Test these cases and capture those tests in code.
An integration test hits the entire system. Go as high as you can and exercise the entire product. If state is needed, it needs to set up and tear down that state. It's okay if ten tests need the same data on disk, but something's got to create that database and then clean it up when the tests are done.
What's the goal here? We want an automated test suite that can be run inside of Cruise Control (http://cruisecontrol.sourceforge.net/) after every code change. With a unit test, no matter who edits "my" code, it'll get retested properly and we'll all know it still works right. With an integration test we'll know the entire system still works properly no matter what we change.
The entire product will stay stable with this type of test coverage in place. And the best time to add it is during development. As you write new code, try to add tests at the same time. The initial coding might take you longer (that's debatable), but you'll make up any lost time by ~not~ having to spend hours debugging "interesting" issues.
Will we miss things that should have been tested? Absolutely. That's why whenever a bug is found, let's add a test that exposes the bug before we fix the problem. Defect Driven Test Creation.
4) Check in code often
Don't hold your code for a long time... days is tolerable, but weeks is Bad. Ideally, try to add one feature or fix one bug, then get a review and push the code. It's okay to get multiple reviews in a day and push code several times a day. In fact, it's ideal.
Just to make the point: Add one feature or fix one bug, then check in the code
If a code push breaks a test, this will ensure it'll be a small chunk of code to back out or or debug.
Getting others to review smaller amounts of code also ensures that the reviewer can actually understand the code. When you ask for a review of 70 pages of intricate code, I suspect the reviewer's eyes will glaze over early.
If you've got a big problem to solve that can't be broken up, then get interim code reviews. Get a single reviewer to go over the new code with you daily. Of course, this much inseparable code might also be a warning sign indicating the code or architecture needs refactoring. ;)
Previous Topic
