Google Engineering Practices

I have recently found out there is a Google Engineering Practices Documentation and learned a great deal from it.

Though the naming indicates it’s related to a broader knowledge about Engineering Practices, it’s actually all about how to create Pull Requests (PR, but Google calls it Changelist, CL) and how to review them.

Specifically, this declaration on their standard for code reviews is the best I’ve seen that describes the concept so on point:

In general, reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.

https://google.github.io/eng-practices/review/reviewer/standard.html

There are two concepts here saying: 1) As long as the PR improves the overall code health; 2) even if it’s not perfect, the reviewer should incline to approve the PR.

I’ve had a recent experience that the PR author was very upset because I rejected their PR twice for some minor changes.

I had my reasons of course, but I did think such conflicts could be prevented if we have a guide like this to follow. Unfortunately I didn’t know such a solid documentation existed before.

Not only just a general guideline like this, the document also teaches you how to review a PR, what to look for and how to approach the process. I’ve been collaborating with other developers for more than 6 years and it’s the first time I’ve learned that there is a recommended order to check the files involved in a PR. It even includes instructions on how to write the PR description.

The main takeaway to me is that it’s not my job to make sure the code in the PR to be perfect as I write it myself.

I sort of had that myth and it was painful to review the other developers’ code, especially if they have a totally different styles than mine.

The best I can do is just leaving review comments and express all valid concerns, admit when it’s a non-block suggestion and give room to the author to have their ownership in part of the codebase.

I think there will still be a long way to go, but for now, I’m more calm when dealing with Pull Requests, thanks to this document.