Skip to main content
Post Made Community Wiki by Mike Roberts
Source Link
Mark E. Haase
  • 5.3k
  • 1
  • 18
  • 12

Both. (Kind of.)

You should be reviewing your own code summarily before committing it. In Git, I think the staging area is great. After I've staged my changes, I run git diff --cached to see everything that is staged. I use this as an opportunity to make sure I'm not checking in any files that don't belong (build artifacts, logs, etc.) and making sure that I don't have any debug code in there or any important code commented out. (If I'm doing something that I know I don't want to check in, I usually leave a comment in all caps so that I'll recognize it during staging.)

Having said that, your peer code review should be generally be conducted after you commit, assuming that you are working on a topic branch. This is the easiest way to make sure that everybody else is reviewing the correct thing, and if there are major problems, then it's no big deal to fix them on your branch or delete them and start over. If you conduct code reviews asynchronously (i.e. using Google Code or Atlassian Crucible), then you can easily switch branches and work on something else without have to separately keep track of all your different patches/diffs that are currently under review for a few days.

If you're not working on a topic branch, you should. It reduces stress and hassle and makes release planning much less stressful and complicated.

Edit: I should also add that you should be doing code review after testing, which is another argument in favor of committing code first. You don't want your test group fiddling around with dozens of patches/diffs from all the programmers, then filing bugs just because they applied the wrong patch at the wrong place.