Re:Code reviews at Google
by curtis on Monday June 07, 2010 @11:01PM
(#1412848)
After my experience with code reviews at Google, I came to the conclusion that Mondrian had some definite deficiencies. On the other hand, later at Mozilla, we did code reviews without any real tool support at all. That was a lot worse. I wrote up some of my thoughts in a blog post which you can find at http://curtisb.posterous.com/what-should-mozilla-look-for-in....
Re:Code reviews at Google
by houseabsolute on Monday June 07, 2010 @11:56PM
(#1412939)
> Mondrian had some definite deficiencies
You mean like its approximately two 8s uptime? :P I do think you're spot on about how Mondrian encourages the nit-picking and doesn't do enough to encourage structural discussions. On the other hand, maybe those discussions should be had before there's any code to review. So . . . meh. Tough problem.
Re:Code reviews at Google
by brg on Tuesday June 08, 2010 @12:44AM
(#1413010)
I found structural discussions took place in the free form areas of mondrian, and knit-picking was a direct result of two aspects:
1) An strict, but oft changed style guideline
2) The requirement to say something beyond LGTM to prove that the code review was thorough.
The one aspect I missed at Google compared to other large software companies was design document reviews. I found that in writing and disseminating such documents a lot of good work was done.
Re:Code reviews at Google
by curtis on Tuesday June 08, 2010 @12:29AM
(#1412984)
I've always found it hard to do something like a structural discussion without having written some code first. At Google I found that I could request an informal code review in email rather than using the formal request mechanism ("g4 mail"). I could even provide a changelist number so the reviewer could look at it in Mondrian. But by having used email I could frame things so the reviewer would look at the high-level picture. That usually seemed to work.
Re:Code reviews at Google
by nostrademons on Tuesday June 08, 2010 @01:14AM
(#1413066)
Mondrian's integrated with e-mail, so I don't really see what the problem is. Oftentimes I'll start a discussion by mailing off a code review with some sketches of how I'll attack the problem, and then the resulting design discussion occurs in an e-mail thread, which is all recorded on the Mondrian code review if I decide to submit the CL.
Another nice feature of that is that if parts of the design discussion pertain to particular features in the code, you can attach them as such, so that the code is automatically quoted in the e-mail thread.
Re:Code reviews at Google
by curtis on Tuesday June 08, 2010 @01:32AM
(#1413099)
Mondrian's integrated with e-mail, so I don't really see what the problem is.
All I'm saying is that it seemed to make a difference how I initiated things.
Another nice feature of that is that if parts of the design discussion pertain to particular features in the code, you can attach them as such, so that the code is automatically quoted in the e-mail thread.
That sounds like either a new feature of Mondrian (it's been 2 years since I was at Google) or one that I simply hadn't discovered (I never completely mastered Mondrian or the code review process when I was there.)
Anyway, my larger point is that at Google there seemed to be a tendency to always default into a low-level line-by-line code review unless I took explicit steps to point the code review into a different direction. I saw a similar tendency at Mozilla. YMMV, of course.
Re:Code reviews at Google
by andymoe on Monday June 07, 2010 @09:35PM
(#1412716)
http://www.fogcreek.com/kiln/ has almost all the features mentioned and they are really really useful. Free for up to two users with the students and startup discount. The free link takes a bit of digging to find if I remember correctly.
Re:Code reviews at Google
by dlsspy on Monday June 07, 2010 @09:40PM
(#1412724)
I've been using gerrit extensively lately. It's a great review system on top of git.
I work with people around the world on software -- including people I've never met who decide to contribute a change (which enters the same workflow as a project lead). It's just awesome.
Re:Code reviews at Google
by AnneTheAgile on Monday June 07, 2010 @09:42PM
(#1412732)
The open source implementation, Rietveld [ http://code.google.com/p/rietveld ], sounds very interesting. Perhaps it could be used in programming classes and might also fit in with pair programming training.
Re:Code reviews at Google
by MarcusA on Tuesday June 08, 2010 @09:35AM
(#1413841)
I see that Rietveld is only a part of Mondrian or at least described as "not the full Mondrian tool". Googlers, what are the differences between the two. Wonder why the full Mondrian tool couldn't be released.
You mean like its approximately two 8s uptime? :P I do think you're spot on about how Mondrian encourages the nit-picking and doesn't do enough to encourage structural discussions. On the other hand, maybe those discussions should be had before there's any code to review. So . . . meh. Tough problem.
The one aspect I missed at Google compared to other large software companies was design document reviews. I found that in writing and disseminating such documents a lot of good work was done.
I wonder if this part of Mondrian has been improved since I was at Google. There was a freeform area, but it wasn't really good for much, as I recall.
Another nice feature of that is that if parts of the design discussion pertain to particular features in the code, you can attach them as such, so that the code is automatically quoted in the e-mail thread.
All I'm saying is that it seemed to make a difference how I initiated things.
Another nice feature of that is that if parts of the design discussion pertain to particular features in the code, you can attach them as such, so that the code is automatically quoted in the e-mail thread.
That sounds like either a new feature of Mondrian (it's been 2 years since I was at Google) or one that I simply hadn't discovered (I never completely mastered Mondrian or the code review process when I was there.)
Anyway, my larger point is that at Google there seemed to be a tendency to always default into a low-level line-by-line code review unless I took explicit steps to point the code review into a different direction. I saw a similar tendency at Mozilla. YMMV, of course.
edit:
kiln: http://www.fogcreek.com/Kiln/StudentAndStartup.html
Better yet sign up for fogbugz and add kiln on to fogbugz:
http://www.fogcreek.com/fogbugz/StudentAndStartup.html
I work with people around the world on software -- including people I've never met who decide to contribute a change (which enters the same workflow as a project lead). It's just awesome.