views:

73

answers:

5

Let me start by describing what our current 'process' is and then ask you how we should improve on it and what, if any, tools you would suggest.

What we are doing currently

Every time someone makes a commit to our svn (or pushes git changes into 'trunk') it ends up in an email in one persons inbox. That person is responsible for glancing over the diff and, if need be, suggest changes / improvements.

This worked pretty well for raising code quality and providing feedback without 'review requests' (those are a different topic and those work pretty well for now).

The Problem

We are approaching 50 commits/pushes a day with > 15 developers and reviewing the diffs isn't really practical anymore, especially via 'clicking a link in an email and looking at the diff in a browser'.

If one person doing the reviews is on vacation that stuff also tends to pile up and there is lots of (imho) useless 'did you check 12345 - 12390 already ?' communication needed when multiple people are working that way.

Questions (finally)

  • Do you think the described process is workable ? (The Team likes it and we want to keep it if it stays - manageable)
  • Are there tools available that could help with that process (on commit automatically create a entry somewhere and everyone can check that as 'reviewed' without any notification, or something, really looking for ideas here ;) )
  • Is this process scalable or will there be the need to dump 'looking at every chance' and move over to some bigger/longer reviews of finished components ?

I hope i made myself clear enough and this qualifies as "answerable question". If i missed something please let me know.


Short summary of what i'm not looking for:

  • Software that allows people to make "review requests" by hand. (While a review board with an api that i could use to automatically create requests would be an option (?) )
  • Maybe not so much: Tools from Atlassian (since they are pretty expensive they would need to offer a great benefit i've overlooked so far, but paying high 5 digit figurers a year will be hard to get trough management so it really has to be worth it)
+1  A: 

You might want to take a look at CodeCollaborator from SmartBear Software.

If you don't feel like shelling out for commercial software, the process you described does sound workable - assuming that the code reviews are randomly distributed to all the members of your team. I couldn't tell if the emails you currently send are sent to the same person every time or sent to a random person on your team, but I hope it's the latter. I would encourage you not to dump this process in favor of only reviewing finished components, because you lose all of the benefits of peer code review.

Edit

In response to the comments: CodeCollaborator has the capability to require pre-commit or post-commit reviews automatically for every commit. Subversion has support for post-commit hooks that would allow you to automatically create a review for every commit. It seems that git also supports post-commit hooks, but I have never used it so I can't say for certain. There may even be a public API according to an out-of-date PDF document, but the links in the PDF document are broken so I would double-check this before buying.

Chris Shouts
He, thanks for the answer. I've looked into CodeCollaborator before but i've only seen the 'enter review request' features and thats not what i care about. I didn't find anything telling me that it is possible to "automatically create review request on commit" or the description of some sort of api that would let me to that. Did i just overlook it ?
edorian
+1  A: 

SVN provides a mechanism that allows you to run scripts on check in. You could use this capability to automatically generate your code review requests on check in.

As the team grows, I'm not sure how scalable this approach overall will be. You're already seeing problems with someone being out of the office when a code review is requested.

There are tools, such as CodeCollaborator, that can help but I'm not sure there is anything that will provide the automation you are looking for.

Scott Dorman
+1  A: 

I've used 2 web-based tools for reviewing:

ReviewBoard, from VMware. Written in Python, its nice and pleasant to look at, and easy to use. Redmine with the Code Review plugin. Written in Ruby. This is possibly the better tool, its got slightly more features and is prettier with lots of edit boxes that can pop up for notes and integration with the redmine trackers.

Redmine integrates with SVN (and all the other OSS scms including git) better as you point redmine at your repo and it generates the diffs on-demand, whereas reviewboard waits for you to generate the diff and send it to it - usually in a post-commit hook. Redmine can also link a review to one of its internal bug trackers so you can easily flag up problem changes. You'll have to hook it up so the review request gets posted automatically (via a post-commit hook again) as a 'bug' in a redmine tracker.

We don't use either in anger, but we intend to. It will almost certainly be Redmine that gets used as the review plugin is pretty nice to use and it has plenty of other good features.

gbjbaanb
+1  A: 

try github.com and their new Pull 2.0 functionality. Basically it allows you keep process similar to one you described, but same time reduce number of pushes/commits into central repository.

In our team we are doing code reviews only when feature/story has been fully developed (in separate branch) prior to merging that into master branch.

Mark Kofman
+1  A: 

My gut assumption is that it wouldn't scale, where I was recently working (new job monday!) there were around 50 programmers in one repository with a commit say, every 5 mins.

In that situation I'd assume it would get out of control (eg. as you've said, people are away, in meetings for hours)

Do you branch? if you did, then perhaps a review (same process you have now) only for when a larger and less frequent commit is done back to the main trunk.

That's my only suggestion though, it's been a while since I've been involved in code review (previous employer should have done it, as well as many other things! -TDD, and deleting 10 year old code for example :)

Soylent Graham