Rietveld, or “How to revamp your code review process”

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.

Now 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…

Rietveld, or “How to revamp your code review process”相关推荐

15个最佳的代码评审（Code Review）工具
代码评审可以被看作是计算机源代码的测试,它的目的是查找和修复引入到开发阶段的应用程序的错误,提高软件的整体素质和开发者的技能.代码审查程序以各种形式,如结对编程,代码抽查等.在这个列表中,我们编制了1 ...
5个开源且简单实用的Code Review工具
更多内容关注微信公众号:fullstack888 Code Review中文应该译作"代码审查"或是"代码评审",这是一个流程,当开发人员写好代码后,需要让别人 ...
如何在团队中做好Code Review
一.Code Review的好处想要做好Code Review,必须让参与的工程师充分认识到Code Review的好处 1.互相学习,彼此成就无论是高手云集的架构师团队,还是以CURD为主的业务 ...
Google是如何做Code Review的？| CSDN原力计划
作者 | 帅昕 xindoo 编辑 | 屠敏出品 | CSDN 博客我和几个小伙伴一起翻译了Google前一段时间放出来的Google's Engineering Practices docume ...
刚进美团，就被各种Code Review，真的有必要吗？
点击关注公众号,Java干货及时送达众所周知,Code Review是开发过程中一个非常重要的环节,但是很多公司或者团队是没有这一环节的,今天笔者结合自己所在团队,浅谈Code Review的价值及 ...
刚入职，就被各种 Code Review，真的有必要吗？
点击上方蓝色"方志朋",选择"设为星标" 回复"666"获取独家整理的学习资料! 来源:juejin.cn/post/ 6882333635 ...
有必要做 Code Review 吗？？？
点击上方蓝色"方志朋",选择"设为星标" 回复"666"获取独家整理的学习资料!作者:梨香链接:https://juejin.im/pos ...
你太菜了，竟然不知道Code Review...
点击上方"方志朋",选择"设为星标" 回复"666"获取新整理的面试资料作者:宝玉来源:http://1t.click/aA4h 我 ...
从零开始 Code Review，两年实战经验分享！
点击上方"方志朋",选择"置顶公众号" 技术文章第一时间送达! 来源:http://t.cn/RtHE14S 前几天看了<Code Review 程序员的 ...
作为CTO，我为什么必须要求代码进行Code Review！
来源:宝玉链接:https://cnblogs.com/dotey/p/11216430.html 我一直认为Code Review(代码审查)是软件开发中的最佳实践之一,可以有效提高整体代码质量, ...

Rietveld, or “How to revamp your code review process”

Rietveld, or “How to revamp your code review process”相关推荐

最新文章

热门文章