How to get developers to do code reviews in a timely manner

The company I work for requires all code to be reviewed by other developers before it is committed. The members of my team are often frustrated because the other developers are too busy coding to do a review, especially if it is very long. How do you incentivize other developers to do timely code reviews?

(We use git-svn so we are able to continue coding while waiting for a review. However, I still find it frustrating when I have to wait a long time before I can commit my code.)

Look at how facebook does it with their own app, called phabricator: http://phabricator.org/

They basically commit on a per issue basis, and for each issue, the code is shown, which is to be reviewed by someone. The code doesn’t go into their main repository until the reviewer said it’s ok to do so.

I guess it makes it more fun.

Also, perhaps a code should be assigned to two people: one who does it and one who reviews it.

Albeit perhaps your teammates don’t believe in this review thingy.

Personally, in lack of reviewers, I used unit tests for lower-level functions and “the janitor test” for all the rest: the janitor test is called that way, because even the janitor should be able to understand your code.

I usually removed some minor parts, like block / function scope brackets, visibility notations, sometimes even types, and showed it to managers, domain experts, mates, whoever requested the code: “is this what you want?”

Also, going there personally and not leaving until reviewing is done helps.

Or, in case you’re not fine with the team, or they’re not fine with you, you know, “if you can’ change the company, change company”…

I’m going to base this on a couple of assumptions:

  1. Everyone seems to want to write code (If not, you have people that need to go.).
  2. Everyone wants their own code to be checked in.

Only allow those who complete their reviews to have their code checked in.

Maybe a certain block of time can be dedicated to code reviews in hopes of avoiding the problem of being interupted.

The goal should be to check in quality code. You don’t want to punish/force the reviews to the point where everyone just gives it a “rubber stamp” approval.

If you have different levels (jr., sr. etc), promotion and maintaining a title should be contingent on doing your job.

At a couple of previous employers, we rotated who was on Code Review on a daily basis. Usually 3 people shared a CR day and each CR had to be signed off by two of the reviewers. This made sure that when it was your day, you knew that CR was expected of you, regardless of your other projects. So every five days or so, it was your turn and you could adjust your development tasks accordingly.

Currently, we have a Team Lead do a cursory CR on their team’s code. Depending on which area of the application has been updated, after the CR is completed, it can be bumped up to the Global Review Team, where we check for things that have a global impact on the application(s), instead of things that are related to a single feature. When there is a large Review to be done, we usually break it up between a few people so no one person has to deal with changes across a ridiculous number of files.

That said, we are only ever reviewing code that’s been committed to the current Dev branch / variant. The code has to pass Code Review, Global Review, DB Review and UI Review (each as needed) before it can be promoted to the next (e.g. Alpha) environment.

Finally, we agree to an SLA on how quickly Reviews are turned around. Hardly anything’s in queue for more than 48 hours and most reviews are done in less than 24 hours.

The company I work for requires all code to be reviewed by other developers before it is committed. The members of my team are often frustrated…

Unless you’re doing mission critical software or critical patches to production release candidate code, there are no compelling reasons to rigidly stick with particular type of code reviews.

  • The idea behind your company requirement sounds reasonable on a surface (100% reviewed code) but the means they decided to use are counterproductive – because as you point out, these lead to developers being frustrated.

Walking in your shoes, I would first point the management that post-commit code reviews are considered equally as respectable as pre-commit ones. These terms are searchable on the web – if needed, find there references to backup this. One does not necessarily need pre-commit reviews to get 100% reviewed code.

Based on above, I would next suggest them to drop micromanagement attitude and let developers try the way that feels more convenient to them. Pre- or post-commit reviews are best left for programmers to choose. If company knows better than programmers, why don’t they write the code themselves?

4

You have a number of issues to address – you have to win hearts and minds and you have to ensure that time is available for code reviews.

The second part is probably easiest – you agree (collectively and that has to include management) that the first thing a dev does each morning is their code reviews – this is simple, understandable, effective and gives you a nice clear stick to beat people with if they don’t comply. Better, you’re not interrupting anything, you’re not asking them to stop working on their code you’re not asking people to squeeze something into their to-do list…

The first part is the real problem – the participants in the review process have to see it as having value otherwise they’ll never do a code review (which is percieved as not having value) when they could be writing code or fixing bugs (which is surely more important…?).

If you can put the two together – firstly ensuring that everyone believes (or understands) that there is value in the code reviews – at the most basic it should mean fewer bugs which means more new code which is usually more fun – and then secondly arranging things so that there is a clear space in the schedule for the code reviews to be done then hopefully good things will happen… it will become part of the culture.

Once its part of the culture it may no longer be necessary to say “first thing every day” – but having said that I think that it fits well into the pattern one probably wants a dev to work in.

4

At most companies I have worked for, you have 3 days to complete a review. It isn’t acceptable to not do the reviews. It’s part of your job. If you don’t do decent reviews on time it affects your performance appraisal. And yes, reviews always seem to happen at the most inopportune times. Too bad, learn to include review time in your estimates. Anyways, if management truly believes that reviews are important (ie. they mandate that all code is reviewed) then they would push a similar policy. Additionally, if people don’t complete the review in the allotted time, that goes as their acceptance of the material.

Consider using a tool like Review Board. It is very helpful, especially for long reviews.

You can upload your diffs and wait until a reviewer has finished their review.
If you have open reviews that prevent you from continuing your work you can report this during your daily meetings (your team want new features to be checked in so that they can be tested as soon as possible, don’t they?).

A few points to add that aren’t in the other answers.

The code to be reviewed must be checked in

  • so that you are reviewing a stable version.
  • It can be on the main development branch if a release is far enough away
  • It can be on branch if there is a good reason not to pollute main

Blocking tasks take priority, therefore code reviews should take priority over other work (but trying not to break your flow).
As a developer you should want others to review your code (as you are aiming to make it better). In that knowledge you should perform reviews for others promptly.

The harder questions are when and how to do code reviews well.

A rule that has worked for us on the when is that shared code must be reviewed as it has wider impact whereas in code for a single application (especially given we are using test driven development) its less important.

Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa Dịch vụ tổ chức sự kiện 5 sao Thông tin về chúng tôi Dịch vụ sinh nhật bé trai Dịch vụ sinh nhật bé gái Sự kiện trọn gói Các tiết mục giải trí Dịch vụ bổ trợ Tiệc cưới sang trọng Dịch vụ khai trương Tư vấn tổ chức sự kiện Hình ảnh sự kiện Cập nhật tin tức Liên hệ ngay Thuê chú hề chuyên nghiệp Tiệc tất niên cho công ty Trang trí tiệc cuối năm Tiệc tất niên độc đáo Sinh nhật bé Hải Đăng Sinh nhật đáng yêu bé Khánh Vân Sinh nhật sang trọng Bích Ngân Tiệc sinh nhật bé Thanh Trang Dịch vụ ông già Noel Xiếc thú vui nhộn Biểu diễn xiếc quay đĩa Dịch vụ tổ chức tiệc uy tín Khám phá dịch vụ của chúng tôi Tiệc sinh nhật cho bé trai Trang trí tiệc cho bé gái Gói sự kiện chuyên nghiệp Chương trình giải trí hấp dẫn Dịch vụ hỗ trợ sự kiện Trang trí tiệc cưới đẹp Khởi đầu thành công với khai trương Chuyên gia tư vấn sự kiện Xem ảnh các sự kiện đẹp Tin mới về sự kiện Kết nối với đội ngũ chuyên gia Chú hề vui nhộn cho tiệc sinh nhật Ý tưởng tiệc cuối năm Tất niên độc đáo Trang trí tiệc hiện đại Tổ chức sinh nhật cho Hải Đăng Sinh nhật độc quyền Khánh Vân Phong cách tiệc Bích Ngân Trang trí tiệc bé Thanh Trang Thuê dịch vụ ông già Noel chuyên nghiệp Xem xiếc khỉ đặc sắc Xiếc quay đĩa thú vị
Trang chủ Giới thiệu Sinh nhật bé trai Sinh nhật bé gái Tổ chức sự kiện Biểu diễn giải trí Dịch vụ khác Trang trí tiệc cưới Tổ chức khai trương Tư vấn dịch vụ Thư viện ảnh Tin tức - sự kiện Liên hệ Chú hề sinh nhật Trang trí YEAR END PARTY công ty Trang trí tất niên cuối năm Trang trí tất niên xu hướng mới nhất Trang trí sinh nhật bé trai Hải Đăng Trang trí sinh nhật bé Khánh Vân Trang trí sinh nhật Bích Ngân Trang trí sinh nhật bé Thanh Trang Thuê ông già Noel phát quà Biểu diễn xiếc khỉ Xiếc quay đĩa
Thiết kế website Thiết kế website Thiết kế website Cách kháng tài khoản quảng cáo Mua bán Fanpage Facebook Dịch vụ SEO Tổ chức sinh nhật