www.codinghillbilly.com   kyle.baley.org  Subscribe / Contact
 
 
 
 
LATEST POSTS
Wednesday, June 23, 2010

Quick PSA today on automating tasks in web apps that don’t have keyboard shortcuts.

I had to migrate about 110 stories from one AgileZen project to another. As yet, AgileZen’s API is read-only so after confirming that the eldest offspring was unwilling and the youngest (being seven months old) was unable, I set up doing the task manually.

(Side note: The reason for the migration is that we were originally using BaseCamp. All our stories were stored as tasks for our beta version and version 1. We had originally moved all the V1 stories into a fake phase for AgileZen just for the sake of getting them out of BaseCamp. At the time, I glanced through the AgileZen API documentation and had mistakenly concluded that I’d be able to migrate them to a new project programmatically when the time came.)

Copying the stories wasn’t too onerous a task. They were all in the same phase and didn’t have any details assigned to them so it was about fifteen minutes of cut and paste from one browser to another.

Deleting the original stories proved a little more monotonous. They can only be deleted by opening the details, clicking Delete, then clicking OK. After doing this manually about five times, I started exploring other options.

On Chrome, I have Vimium installed. It gives you Vim navigation keyboard shortcuts, similar to Vimperator for Firefox. With it, when you can “click” links by pressing f, then typing the appropriate code corresponding to the link. That means I can handle the clicking involved to delete a story from the keyboard.

I also have AutoHotKey, which is designed to automate keystrokes and mouse clicks. I use it to create keyboard shortcuts like Win+n for notepad and Win+g to open the hillbilly community forums website and an alternate way to close an app without using Alt-F4.

Between the two of them, I came up with this quick ‘n dirty script to delete stories:

#q::
Delete()
return

Delete() {
  Send f
  Sleep 250
  Send s
  send h
  Sleep 1000
  Send f
  Sleep 250
  Send a
  Send f
  Sleep 500
  Send f
}

Now, when I press Win+q, the following happens:

  • Press f to tell Vimium I’m going to navigate to a link
  • Pause a quarter of a second; without this, the script goes too fast for Vimium
  • Enter the key code for the story I want to delete. Note that this is always the top story in the phase so the code is always the same as long as there is at least one story in the phase
  • Pause a second to give the browser time to navigate to the story
  • Press f again for Vimium navigation and pause
  • Enter the key code for the delete button
  • Pause half a second to give the confirmation dialogue time to appear
  • Press f again for Vimium navigation

At this point, I enter the key code for the OK button manually just to be on the safe side.

Pausing for a second to let the browser navigate to the story details is a minor hack. I’m pretty sure I could get AutoHotKey to wait until the title of the window changes or something else more accurate. But I have to balance out the time it takes to figure this out versus the time it takes to do it manually…ignoring the time it took to write this post, of course.

Kyle the Served

Tuesday, June 22, 2010

I have a new mistress and her name is Rietveld. I call her Veldy or sometimes, in the throes of passion, RV. But only because I have no idea how to pronounce Rietveld out loud.

In a recent post, I talked about our switch to the code review tool. And by recent, I mean less than a month so it’s been a torrid affair but I can tell she’s the code review tool I’ve been waiting my whole career for.

In that post, I described our old process and the proposed new one. Now, a few weeks later, I can begin my mission to make sure every man, woman, and child knows about the tool whether they do code reviews or not.

Our process is the same as outlined in the previous post but I’ll mention some side notes here just to ensure there’s new content in an attempt to keep potential trolls at bay.

The absolute greatest feature by far of the tool is how it enables asynchronous and disconnected reviews. By “disconnected”, I mean I don’t need to have the code with me. (You still need an Internet connection.) I just finished a ten day trip to London* and performed at least half a dozen reviews during that time on my wife’s laptop. Everything I need is included in the review page: the old code, the new code, and any comments by other reviewers or the initiator.

There are a couple of things this has enabled. First, I can do code reviews any time I have a free moment. I whipped off at least four this morning. Before I hit the sack for the evening, I’ll often open Rietveld up and bang out one or two just to end on a high note. It’s rare that I spend more than half an hour on any of them. More often, it’s closer to fifteen or twenty minutes.

If you’re wondering why we have so many reviews, that’s the second “enabled thing”: It’s so easy to do them, we can now review everything. We even have our own process swimlane for stories that are to be code reviewed. I.e. a story can’t be marked done until it’s passed the review. Ditto for bugs. The reason this works? Because it’s trivial to start a review and it’s almost trivial to do them. At this given moment, we have fifteen active code reviews. I expect at least three of them to be closed in the next twelve hours. But I’m also expecting two more to be created in that same time.

And because it’s asynchronous, I don’t even need to finish a review when I start it. Any comments I’ve made that session are stored as drafts until I publish them. I’ve done this a couple of times: Hit some code that required more thought than I was able to give at the moment so I just shut down the review and moved on to the next one.

There is a nice side effect that’s come from this process. Our team consists of one junior developer, a senior developer, and me. Because the comments are persisted and because we don’t allow stories to continue until the reviews are complete, the reviews have become a good source of training and mentoring for the junior dev. She will submit a review for a story, launch a review, and then update the code several times addressing issues that were brought up during the review. Sometimes the comments are trivial, like “make sure the indentation is consistent”, sometimes it involves a fairly sizeable chunk of work, like “add unit tests to expose the bug”. During this time, she can submit further patch sets and Rietveld can show you the diffs between them and the previous patch set. All the while, we (and by “we”, I almost always mean “the senior dev”) are providing on-the-job mentoring that’s relevant and contextual.

“Code review” no longer holds a connotation of six people in a room for hours being kept conscious by whatever substance passes for coffee these days. Now, it’s become a relatively benign task, like verifying we have unit tests. Story’s done; launch a review and move on to the next one.

So if you meet the requirements (use SVN, Hg, or Git; can create an app on AppEngine), you’d make an old-ish Hillbilly mighty happy iffen you’d check it out.

Kyle the Renewed

Wednesday, June 02, 2010

For me, code reviews have traditionally been one of those aspects of software development that fall into the “good idea; bad execution” category. It’s something that I know we’re supposed to do but when someone says, “let’s do a code review”, my first reaction is usually to make a cross with my fingers and start yelling “THE POWER OF CHRIST COMPELS YOU!” at them.

Unfortunately, our team is scattered across three time zones these days so that tactic is less effective than it has been in the past. Which means we’ve had to address the real problem: how do we make code reviews useful?

Our first attempt was to do them the tried-and-almost-true way. That is, we scheduled a code review meeting on Wednesdays and I’d send out notices to the other developers on Monday saying we’re reviewing these five classes. I sent the notes out on Monday to give people plenty of time because it wouldn’t be realistic to expect them to do the reviews an hour before the meeting starts now, would it…

Historically, code review meetings that I’ve attended/led have usually followed a familiar pattern. To summarize:

Meeting starts
Coding Hillbilly: Okay folks, I’m pumped to get started. There are some interesting things going on in this code and some awesome comments. Let’s take an easy one to start off: “This code is pretty coupled to the implementation. We should create an event bus to manage the communication between the various presenters.” I agree and let me tell you at length why I say that…

End of Hour One
CH: I’m going to have to cut off your overview of dependency injection frameworks, Arbuckle. You’re right, I think that will help us with the internationalization of our config files. But we’ve only covered three issues so far. The rest of them should go much quicker though. Let’s take a look at the next comment…”This should be split out into a REST service”…

End of Hour Two
CH: Moving on…”We shouldn’t need a separate interface for this. It’s just extra complexity. Just create a new instance of the implementation in the constructor.”………<sigh>…Agreed. Next…

End of Hour Three
CH: Next…”Move the hard-coded database connection string out of the vie---“…you know what? The rest of this code looks good to me. Any objections if we accept it as-is? Great. See you all next week.

In this little slice of life, I’ve also ignored an important aspect of code reviews with remote teams. In an attempt to keep comments as close to the code as possible, they are done as //TODOs inline with the code. And to avoid annoying merge issues, this means everyone needs to do the actual reviewing at different times. So there’s a lot of “Okay, I’m done. Have at ‘er” IMs thrown about.

As much fun as this is, I decided to give Rietveld a look. I stumbled across it when I noticed Philippe Beaudoin, the creator of gwt-platform, was using it for his project. It was easy to pick his brain about it because, y’know, he’s on our dev team.

Some notable tidbits about Rietveld. It’s based on Mondrian, the internal code review tool used by Google. And it was created by Guido van Rossum. As a general rule, if the creator of a piece of software has also written a widely used programming language, it’s worth a quick eval.

But it was the workflow of Rietveld that drew me in. This is our new code review process:

  • A developer initiates a code review by selecting a starting revision and an ending revision. The diff between the two is submitted for review to the selected reviewers
  • The reviewers look at the diffs (either as a side-by-side diff or, if you prefer a challenge, a unified one) and draft comments inline with the code on the web app (i.e. not in the code in the repository itself)
  • When a reviewer is done drafting comments, she publishes them to other reviewers and the initiator
  • The initiator reviews the comments, makes changes if necessary, and publishes updated code diffs in response to the comments (again, if necessary)
  • The initiator and other reviewers can keep adding comments back and forth. The comments are stored in a discussion format similar to how they appear in GMail.
  • Once all comments have been addressed (either through code or discussion), the code review is marked closed.

Here are the parts I love about this:

The reviewed code is a diff

We don’t need to review an entire class every time. We just look at what’s changed from one revision to the next. So we can focus on the functionality for a specific bug or user story. This makes it easier to implement a policy like “User stories must be code reviewed before they can be marked complete.”

At the beginning, this will be a pretty moot argument since diffs will include a lot of new classes. But even still, we can see *all* the changes that have been made for a particular story, including that “minor” change you made to the GetUsersTimeZone function in the global DateUtils class.

Review comments are an ongoing discussion

The comments are threaded in the interface and capture more of the discussion than our previous process, where one developer would add a //TODO and the rest of the discussion was done at a meeting. And speaking of the code review meeting…

NO CODE REVIEW MEETING!

This is by far, my favorite aspect of Rietveld. Once launched, code reviews are done asynchronously. Reviewers add comments whenever it is convenient (within a reasonable timeframe, of course). Other reviewers can chime in when they want. Once published, comments are immediately seen by other reviewers and can be commented upon further. For a disparate team like ours, this feature alone is pure 100 proof homemade moonshine. And finally:

The code review isn’t done after the meeting

In our previous process, the code review was all but forgotten after the meeting. There was no inherent follow-up process to make sure the changes had been made. Here, the code review remains active until it is explicitly closed. I.e. once all comments have been addressed to the team’s satisfaction.

sally-code-review.pngNow to implementation details. Rietveld is a Django app running on Google App Engine. There’s a publicly hosted implementation of it at http://codereview.appspot.com. There, you’ll find a truckload of OSS apps registered for it (and in fact, Rietveld is open source itself if you feel like looking at Python code written by the creator of Python). Nice thing about that is that you can see examples of code reviews in progress.

If, like us, you are leery about publishing your reviews publicly, you have at least two options that I know of to host it yourself. One is to create an application for yourself on App Engine and host it. There are instructions to do that and I’m sure they work. We didn’t try them because the other option is much easier. If you have a Google Apps For Your Domain account, you can add it directly from the Google Apps Marketplace. One click and it’s added to your account and only you and your domain users have access to it.

From what I’ve read, Rietveld works with Subversion, Mercurial, and Git repositories. From experience, they can be private repositories as well as public ones.

It’s probably too early in the process to be extolling the virtues of our new code review process. At this stage, Rietveld is kind of a rebound girlfriend for us because of the pain we felt with the old process. But early indications are that it will be a big hit.

If not, I suppose we could print out the code and fax comments back and forth…

Kyle the Encoded

Disclaimer
The opinions expressed herein are my own personal opinions and do not represent my employer's view in any way.

Copyright © 2010 Kyle Baley. All rights reserved.
 
CATEGORIES
.NET General (18) alt.net (4) altnetconf (9) ASP.NET AJAX (40) ASP.NET MVC (29) Bahamas (1) Bahanet (9) BDD (1) BookedIN (1) Brownfield (21) Career (10) Castle (1) Code coverage (1) Code review (2) Coding Style (6) Communication (1) Community (18) Conscientious Coding (36) Continuous Integration (11) dasBlog (12) Development (16) DevTeach (4) Domain (2) Environment (4) Estimating (1) Featured (14) Flamingo (10) Games (1) Google App Engine (4) GWT (10) Hardware (6) Java (2) Javascript (7) Linq (3) Livelink (6) Lucene.NET (2) MbUnit (1) Metrics (2) Miscellaneous (25) Mocking (4) NAnt (4) NHibernate (13) NInject (1) Office (3) Office Development (6) Open Rasta (1) Patterns (6) Presenting (14) Professional Development (15) Refactoring (10) ReSharper (11) REST (3) S#arp Architecture (5) Security (3) Software (11) Sundry (19) TDD (19) Tools (23) User Interface (6) Utilities (9) Visual Studio (8) VSTO (1) Web development (12) Windows (3) Working Remotely (18) Workplace (3) Writing (6)
 
LATEST POSTS
 
POPULAR POSTS
 
 
ARCHIVE