Currently I'm working on a big project refactor and we are doing code review in a daily basis. To be honest, I hope there is a standard tells me how to do this including: what you should do when post your change, and even how do you perform a code review to others.
Thankfully I got this Google instruction on Google. I will share some thing I learned from these article series.
Termnology
- CL: Stands for "change list", which mean self-contained change that has been submitted to versio control or which is undergoing code review. Some organizations also call this a "change", "patch" or "pull request(PR)".
- LGTM: Stands for "Looks Good To Me". It is what a code reviewer says when approving a CL.
What Do Code Reviews Look For?
Picking the Best Reviewers
Google suggests we should find someone that could respond in a reasonable period of time.
The best person is owner of the code. I thinks you can get the name from git blame. If they are not availabe, at leart CC them on you change.
I usually invite the code owner, my mentor(a senior engineer), sometimes my manager and another engineer(usually a senior or principal engineer).
In-Person Reviews(and Pair Programming)
I never did a pair programming though.
I like in-person code review which enable reviewee to share the whole idea of design or details of a particular code. This would help me to explain my code more clearly to reviewers. On the other hand, reviews has a better understanding on codes and you will discuss in the flash once there is a question.
The CL Author's Guide
Writing Good CL Descriptions
- First line use imperative sentence to summarize what you have done in this change.
- Followed by a blank line and a description, inclduing what problem you are trying to solve, what approach you are using, possible shortcomings and optional background infomation.
Small CLs
- Small means this CL only address one thing.
- Should include test code.
How to Handle Reviewers Comments
- Don't take it personally. Focus on the code part and that's you main task.
- Fix the code. Try to clarify first if reviewers express consusion.
- Think Collaboratively. No matter you will change your code or not, communicate with reviews about the solution.
How To Do A Code Review
(For this part I will take time to quick go through the Googe guide and share some ideas once I've done some CRs.)