I just found this on Slashdot – a guy named Fabien Sanglard reviews the code for the iPhone DOOM port. This is cool – I wish I had time to read it thoroughly. It’ll have to wait until after my assignments are done.
Looks like this guy has a thing for FPS’s – he’s also done a code review for “Wolfiphone” (Wolfenstein for iPhone) and the original Quake.
I like this idea. Why aren’t there more of these on the web? Or am I not looking hard enough?
So, for my Master’s thesis, I’ve pretty much set my heart on code review as my research area. Here’s why: it works. It makes your code better. It helps you find bugs. And I’m not just quoting something overheard in a pub – there’s evidence to back these claims up.
And then there’s my own experience to boot – the MarkUs team has been using ReviewBoard as our pre-commit code review tool since last summer, and I wouldn’t ever go back. If I ever have to work in a shop that doesn’t perform code reviews, I’ll campaign my butt off.
Having said all that, pre-commit reviews certainly aren’t for everyone. Some downsides of pre-commit over post-commit:
There are probably more.
My response: at least for MarkUs, pre-commit code reviews are working just fine, thank you very much. At least we’re reviewing it – and any review is better than no review. But to continue my response, here are a couple of advantages of pre-commit code review for the MarkUs development team:
Yep, there’s just something so satisfying about seeing all of those little green “ship-it’s”, and then firing off your code into the repository… it’s positive reinforcement for code reviews. And it’s strangely addictive to me.
A little while ago, I wrote about what I consider to be one of the Achilles’ Heels of Peer Code Review. Here’s another one: at least for ReviewBoard, during a review all you’re looking at is the code. That’s all fine and dandy if you want to look at the logic of the code…but what if you want to try it? Does trying it out help find more bugs than just looking at it?
Well, at least for MarkUs, it’s helped. I’ve recently started checking out a fresh copy of MarkUs every time a review request is put up, and splat a copy of the diff under review on top. I run the test suites, and if they pass, I drive it around. I try out the new features that the diff supposedly adds, or try to recreate the bug that the diff supposedly fixes.
And I’ve caught a few bugs this way. This is because ReviewBoard is good at showing me what is in the code, but is bad at telling me what is not there. And that’s perfectly understandable – it’s not psychic.
So here’s an idea – how about writing a little script that checks ReviewBoard for new review requests. When it finds one, it checks out a brand new copy of MarkUs, splats down the diff under review over top, runs the tests, and then posts back as a ReviewBoard reviewer how many tests passed, how many failed, etc. If we wanted to get fancy, the script could even do some commenting on the code – maybe using Roodi, Flog, Flay, or some of those other sadistically named Ruby tools to say things about the diff. The script would be another reviewer.
And then the kicker – the script posts a link in its review where developers can try out a running instance of MarkUs with the applied diff.
Want a fancy name to kick around the office? Call it pre-commit continuous integration. I just checked – it’s not a common term, but I’m not the first to use it. Again, so much for being cutting edge.
Would this be useful? It’s possible that the Roodi/Flog/Flay stuff would bring too much noise to the review process – that’s something to toy with later. But what about the link to the running instance? Will that little feature help catch more bugs in MarkUs? How about for Basie?
I’m curious to find out.
Unfortunately, ReviewBoard doesn’t let me download diffs through its API just yet…if schoolwork lets up for a few days, I’ll look into changing that.
I’d love to hear your thoughts.
Belated happy holidays! My last post was over a month ago, and so my blog has a nice layer of web-dust on it right now. Well, here I am to ease your mind. I’m still alive!
But that almost wasn’t true.
I won’t bore you with the details – I’ll just give you the facts, and let you fill in the blanks.
Breakfast tasted especially good for us that morning.
Anyhow, now where was I? Oh yeah…
MarkUs 0.6 got kicked out a week or so ago. The MarkUs Team kicked the crap out of a bunch of tickets over the holidays, and I think we ended up with a pretty solid release. MarkUs is being used again at UofT this semester, and Byron Weber Becker is also piloting it at UWaterloo. I’ll cautiously say that things seem to be going well for this release. Great job, MarkUs Team!
I’m TAing the students working on MarkUs for Greg’s UCOSP course again. We had a fantastic code-sprint this past weekend! The new team members have already started working on tickets and submitting code to review. I think we’re on our way into another highly productive semester.
Remember that big list of code review tools I put up a while back? I’ve got a few more to add:
This is a pretty dead-simple code review tool that came about during a Rails Rumble a few months back. It has that “big friendly buttons and round corners” web 2.0 thingy going on. I haven’t gone so far as to actually try it out, but I did watch this web-cast:
Not bad if you just want to get your code out there, and get your team commenting on your changes…
A few things caught my attention:
Still, if I was working on a project hosted on a Git repo, and I needed a dead-simple code review service, and I needed it quickly, I could probably do a lot worse than this.
Click here to check out How’s My Code
Remember that time when I wrote about how it might be neat if somebody created a code review tool on top of Google Wave? (or Bespin for that matter – though I didn’t mention it, and should have)
Looks like somebody else was thinking the same thing. And a few months earlier. I guess it’s not easy to be super cutting-edge.
Anyhow, looks like something Wave-ish (yet simpler, more streamlined) has been developed. Check out Squad.
I just tried this thing out for free (with ads, features locked, etc), and it was pretty cool. I could see something like this being very useful for showing new MarkUs team members how to do things. Actually, I just used it to show a new member of the MarkUs team how to use Shoulda. Pretty useful. It sure beats coding through IRC and Pastie.org.
A few things to keep in mind:
I can see this becoming second nature, like Pastie.org.
Who knows – I might find more reasons to use Squad as the semester rolls, and MarkUs picks up speed. I’ll keep you posted.
If you missed the link I put in above, click here to check out Squad
This service crowd-sources code review requests, so don’t expect to get deep architectural feedback, because it’ll probably come from strangers who don’t/barely know your code base.
The idea is – slap a piece of code that you’d like refactored up on the site, and then others swoop in with brilliant suggestions (assuming of course, you asked your question properly…check this out…what the…?)
This is the sort of thing that CS instructors probably wouldn’t want their students using too much…it’d then become solve-my-CS-programming-assignment.com.
Still, I think it counts as peer code review. And it’s way different that anything else I’ve been looking at. Nice.
Click here to check out Refactor My Code
Anyhow, I just thought I’d mention those.
If you haven’t already heard about Google Wave, watch this video.
In a nutshell, Google Wave is an attempt at integrating wikis, instant messaging, email and social networking, into a nice, tight, Google-y ball of goodness.
What’s particularly interesting, is that developers can write “gadgets” by coding against the Google Wave API. Check this out – a collaborative tool for systems modeling built into the Google Wave client. That’s pretty cool!
Which makes me wonder… could a code review tool be built on top of Google Wave? And if so, what services, if any, does the Wave protocol offer that might make such an application superior to other web-based code review tools?
What does Google Wave offer that would make such a code review tool special, more effective, and better?
Two more ideas to augment the code review process:
Imagine you’re reviewing a piece of code. There’s something like…500 lines to look at. So you’re scrolling through the diff, checking things here, checking things there…
And in the background, the review software is recording which sections of the code you’re looking at.
Hm.
I wonder if it’d be a useful metric to know what portions of the code were being looked at the most during a review? I believe it’s possible (at least within Firefox) to determine the scroll position of a window. I believe it’s also possible to determine the dimensions of the entire diff page, and the offset coordinates of elements on that page.
So shouldn’t it be possible to determine which elements were on the screen, and for how long?
Imagine seeing a “heat map” over the diff, showing where the reviewer spent most of their time looking.
Jason Cohen wrote that code reviews should take at least 5 minutes, and at most 60-90 minutes. I wonder if collecting this information would help determine whether or not a review was performed carefully enough.
Now, granted, there are plenty of ways to add noise to the data. If I’m doing a review, and stand up to go get a sandwich, and then my neighbour visits, etc…my computer sits there, gazing at those elements, and the data is more or less worthless…
Which brings me to:
Eye tracking is not a new technology. One of my roommates works at a lab where they do experiments with eye tracking tools. From what he tells me, the eye tracking gear in the lab is pretty expensive and heavy-weight.
But check this out:
and this:
and this:
So it looks like a webcam and some clever software can do the eye tracking trick too. This stuff is probably far less accurate than what my roommate uses – but it’s lighter, and cheaper, and therefore more likely to find its way into homes. So it looks like the day is coming where I’ll eventually be able to use my eyes to control my mouse cursor.
But, more interestingly, this more or less solves the problem with my Screen Age idea: this technology can tell when you’re looking at the screen. And it can give a pretty good guess about what area of the screen you’re looking at.
I wonder if collecting this information from code reviewers would be useful – what exact parts of the code have they looked at? And for how long? What have they missed? What did they gloss over?
UPDATE: Karen Reid has also brought to my attention the possibility of using this technology to see how TAs grade assignments by tracking what they’re looking at. Hm…