How It Works
While working on a new feature, Dave (for example) will cut a branch from the current version of our master product and work exclusively on that branch, a process with which I’m sure most of the coding world is intimately familiar. But before he can reintegrate the changes into master, Pratik or another qualified engineer must review his work and give him the stamp of approval:
LGTM (Looks Good To Me)
If Pratik has an issue with the way Dave has designed or coded the work, they’ll have a discussion (potentially with a long volley of back-and-forth reasoning) until they reach an agreement. Or, if Pratik has no issues, he can LGTM the work right away.
1) The primary reviewer is the author i.e. YOU.
2) Create a checklist for yourself of the things that the code reviews tend to focus on. Some of this checklist should be easy to put together. It should follow the outline of the coding standards document. Because it’s your checklist, you can focus on the thing that you struggle with and skip the things that you rarely, if ever, have a problem with. Run through your code with the checklist and fix whatever you find. Not only will you reduce the number of things that the team finds, you’ll reduce the time to complete the code review meeting—and everyone will be happy to spend less time in the review.
3) You are not your code. Remember that the entire point of a review is to find problems, and problems will be found. Don’t take it personally when one is uncovered.
4) Understand and accept that you will make mistakes. The point is to find them early, before they make it into production. Fortunately, except for the few of us developing rocket guidance software at JPL, mistakes are rarely fatal in our industry, so we can, and should, learn, laugh, and move on.
5) No matter how much “karate” you know, someone else will always know more. Such an individual can teach you some new moves if you ask. Seek and accept input from others, especially when you think it’s not needed.
6) Don’t rewrite code without consultation. There’s a fine line between “fixing code” and “rewriting code.” Know the difference, and pursue stylistic changes within the framework of a code review, not as a lone enforcer.
7) The only constant in the world is change. Be open to it and accept it with a smile. Look at each change to your requirements, platform, or tool as a new challenge, not as some serious inconvenience to be fought.
8) Fight for what you believe, but gracefully accept defeat. Understand that sometimes your ideas will be overruled. Even if you do turn out to be right, don’t take revenge or say, “I told you so” more than a few times at most, and don’t make your dearly departed idea a martyr or rallying cry.
9) Don’t be “the guy in the room.” Don’t be the guy coding in the dark office emerging only to buy cola. The guy in the room is out of touch, out of sight, and out of control and has no place in an open, collaborative environment.
10) Please note that Review meetings are NOT problem solving meetings.
11) Help to maintain the coding standards. Offer to add to the coding standards for things discussed that aren’t in the coding standards. One of the challenges that a developer has in an organization with combative code review practices is that they frequently don’t know where the next problem will come from. If you document each issue into the coding standards, you can check for it with your checklist the next time you come up for code reviews. It also will help cement the concept into your mind so that you’re less likely to miss opportunities to use the feedback.