Comment by Mark on "How code reviews work at Microsoft"

After this review cycle, reviewers mark the code as okay, and Rose can finally check-in the code into the common code base.

I'm kind of surprised that pre-commit reviews are apparently the default at Microsoft.

In my experience having done both, it's so much easier to just review pull requests (so after commits).

The tooling is easier (the diffing and building and static analysis that is mentioned, for example, can just be done on the code in the branch). Manually attaching test results to reviews sounds really 2005.
It is much easier to guarantee that all code is reviewed. If uploading code to review and committing it are separate steps, you'll need some validation.
You can continue making related changes on the branch while review is ongoing. You can also pull in changes from master, and they won't pollute the review (maybe CodeFlow can understand this; I used another pre-commit review tool that couldn't, while it's easy with git).
Committing code regularly means you don't need local backups on your development machine.

It's such a big company and they apparently analyzed the review process in quite some detail. I wonder if I am missing something, or Microsoft is.

Despite my disagreement with this aspect, this is a useful series with useful information! Made that bold to emphasize it as the tone of this comment is quite negative despite me liking the series :-)

Hi Mark, thank you for the comment. Let me clarify a few things: First, the code review is not pre-commit. The developer commits probably several times to the private development branch. But many teams have the policy to require a code review before the code is merged with and checked-in to the common code-base, i.e., shared team branches and Master branch for example. If I understand your comment correctly, this is similar to what you explain. Using Git, this would mean to do a code review for the pull request.

But, at Microsoft, there isn't one code repository system. There are several different in use, depending on teams and products. So, I explained it more high-level to cover all different scenarios. I'll update this passage to clarify it better. Also, as you can imagine, the branch structure for teams with several thousand engineers working on one product can be quite evolved.

There are some teams that allow code reviews after committing to the shared development code base. So, they allow developers to check-in their code in the shared development branch before and review it later. If this is allowed depends on the team and the kind of change.

About the other points. Yes, CodeFlow, and also other code review tools used at Microsoft are advanced and it guides and helps with many activities. There is also no need to manually attach test results. That is all done for you.

Obviously, with so many developers around, things get automated. So, if there are different steps involved, you can be assured they are conveniently designed to ease the job of the developer. As there are so many teams what exactly, and how it exactly is done differs. I cannot go into too much detail here. But, it is definitely not 2005 ;-)

I hope this clarified your questions and thanks a lot for your comment. Sometimes it is hard to describe processes I am so familiar with. So, your questions and comments help me to better describe the process.

Search Hashnode