原文
google/eng-practices: Google's Engineering Practices documentation
自分が重要だと思うポイントだけを記載します。
レビュアに対して
1 完璧なコードなど存在しない
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.
コードがシステムの質を保つ前提で、マージできる。→ベースライン。
一発の完璧を追求するのではなく、継続的な完璧を追求し続けるべき。
品質を保つと同時に、被レビュー者のモチベーションも維持することも大事。
2 コードレビューは教育の場でもある
It's always fine to leave comments that help a developer learn something new. Sharing knowledge is part of improving the code health of a system over time.
チームメンバー同士のお互いの成長のため、言語やフレームワークに対する知識をどんどん共有し、コメントしましょう。
3 スタイルを統一するべき
The style should be consistent with what is there. If there is no previous style, accept the author's.
The author of the CL should not include major style changes combined with other changes.
スタイルは常に統一するべき。また、スタイルガイド（docなど）は絶対的である。
もしガイドになかった場合、被レビュー者のスタイルを採用することを推奨。
スタイルのみを修正する場合に、独立したMRを出すべき。
4 意見を統一できない場合
In any conflict on a code review, the first step should always be for the developer and reviewer to try to come to consensus
意見統一できない場合に、当然レビュー自体がうまく進められない。
対面のMTGでお互いの意見を話し合うか、それでもダメな場合に上長と合わせて議論しましょう。→もちろん、結論をMRとか、Docとかに残す必要がある。
5 コードの複雑さを注意する
"Too complex" usually means "can't be understood quickly by code readers." It can also mean "developers are likely to introduce bugs when they try to call or modify this code."
ぱっと分からないコードがあったら注意すべき。
特にオーバーエンジニアリングに対して、避けるべき。
6 テスト
Will the tests actually fail when the code is broken? If the code changes beneath them, will they start producing false positives? Does each test make simple and useful assertions? Are the tests separated appropriately between different test methods?
Remember that tests are also code that has to be maintained. Don't accept complexity in tests just because they aren't part of the main binary.
テストは大きいテーマでもありますが、
最小限では、テストの有効性（不具合発見やリファクタリング）や複雑性を注意するべき。
7 全ての行を確認する
In the general case, look at every line of code that you have been assigned to review.
If it's too hard for you to read the code and this is slowing down the review, then you should let the developer know that and wait for them to clarify it before you try to review it.
一部のコードが理解していない場合に、被レビュー者に説明してもらう権利がある。自分が理解していないことが、他のレビュアも理解していないかもしれないからです。
一行一行をしっかりチェックしてあげましょう。
8 褒めること
If you see something nice in the CL, tell the developer, especially when they addressed one of your comments in a great way.
良くない部分を指摘することがもちろん大事だけど、褒めることが思ったより効果的である。
9 早いレスポンス
Most complaints about the code review process are actually resolved by making the process faster.
大半の文句は早いレスポンスだけで解消できる。
一回の修正を小さめにして、レビュアのレスポンス（コメントやサムズアップ）をスピードアップできれば、開発者に対しても、レビュアに対しても良循環になりやすくなる。
10 MRが大きい時
Even if it sometimes takes a long time to get through the entire review process, having quick responses from the reviewer throughout the process significantly eases the frustration developers can feel with "slow" code reviews.
If somebody sends you a code review that is so large you're not sure when you will be able to have time to review it, your typical response should be to ask the developer to split the CL into several smaller CLs that build on each other, instead of one huge CL that has to be reviewed all at once. This is usually possible and very helpful to reviewers, even if it takes additional work from the developer.
レビュアのレスポンスが遅い場合に、開発者のモチベーションが下がりうる。
逆に開発者のMRが大きい場合に、レビュアがMRの分割を要求することができる。（開発者に対して負担をかかることがあるが、経験的にやりがいがあることだと分かる）。
どうしても分割できない場合に、レビュー前にコメントを入れることがおすすめ、開発者は常に着手できるような状態になるべき。
11 サマリ
Be kind.
Explain your reasoning.
Balance giving explicit directions with just pointing out problems and letting the developer decide.
Encourage developers to simplify code or add code comments instead of just explaining the complexity to you.
礼儀正しく。
コメントする理由をできるだけ詳しく。
方法性だけを示すか、それとも正しいコードを提供するかのバランスをとること。
レビュアがコードを理解できない時に、よくあるのはコードが読みづらいことなので、コードを修正するか、コードにコメントをつけるかにするべき。（レビュー時に説明してもらってもその場のレビュアしか理解できないため、将来的にまた同じ問題が別の開発者が遭遇する）
12 開発者からの反抗
When a developer disagrees with your suggestion, first take a moment to consider if they are correct. Often, they are closer to the code than you are, and so they might really have a better insight about certain aspects of it.
However, experience shows that as more time passes after a developer writes the original CL, the less likely this clean up is to happen. In fact, usually unless the developer does the clean up immediately after the present CL, it never happens.
Sometimes it can take months for these complaints to fade away, but eventually developers tend to see the value of strict code reviews as they see what great code they help generate.
通常では開発者の方がコードに詳しいので、もし違う意見が出たら開発者を尊重したうえでゆっくり議論しましょう。
今やらないことが別issueで起票しても、すぐ直さない限り永遠に直さないとよくあるでしょう、できるだけ修正できるものを修正しましょう。
厳しいレビューの場合に、開発者は最初段階結構苦労するけど、長期間で見れば絶対にいい話に間違いないです。
被レビュー者（開発者）に対して
1 良い説明（Overview）を書こう
A CL description is a public record of what change is being made and why it was made. It will become a permanent part of our version control history, and will possibly be read by hundreds of people other than your reviewers over the years.
レビュー実施時にレビュアに説明できるけど、
レビュー後のことを考えると、他のメンバーがコードを読まずに分かるような最小限な説明を書くべき。
説明の中に、一番重要な一行目の文書をより丁寧に書くべき。
2 Small MR
Reviewed more quickly.
Reviewed more thoroughly.
Less likely to introduce bugs.
Less wasted work if they are rejected.
Easier to merge.
Easier to design well.
Less blocking on reviews.
Simpler to roll back.
レビューしやすい、早い
レビューを徹底できる → でかいMRであるほど、いろんな小さいところを注意できなくなる
不具合を少なくする
無駄な作業の繰り返すことを避けられる
マージしやすい → コード量が少ないので、当然衝突が少なくなる
デザインしやすい → でかいMRの場合、どんどん修正したくなくなる
レビューがボトルネックになりにくい
ロールバックもしやすい → 影響範囲がでかくなるとロールバック自体が大変になる
3 Small MRの定義
In general, the right size for a CL is one self-contained change.
There are no hard and fast rules about how large is "too large." 100 lines is usually a reasonable size for a CL, and 1000 lines is usually too large, but it's up to the judgment of your reviewer.
Reviewers rarely complain about getting CLs that are too small.
一つのMRは一つのことしかやらない。
テストコードが必須。
Developにマージする場合に、システムが正常に動けること。
100行が理想、1000行以上なら良くない。
通常、レビュアは小さすぎるMRに対しても文句言わない。（小さすぎるの心配不要
自動生成のコードと削除されたコードはカウントしなくても良い。
MRをどうしても分割できない場合に早めに相談すること。
4 テストコードについて
A CL that adds or changes logic should be accompanied by new or updated tests for the new behavior. Pure refactoring CLs (that aren't intended to change behavior) should also be covered by tests; ideally, these tests already exist, but if they don't, you should add them.
新しいロジックを書く場合に必ずテストを書くこと。
リファクタリングの場合に必ずテストを書くこと。
既存のコードを修正する場合に、もしテストがない場合に追加してあげるべき。
5 コメントに対する返事
Sometimes reviewers feel frustrated and they express that frustration in their comments.
Never respond in anger to code review comments.
Don't Take it Personally
レビュアがコードを理解できない時にどうしても感情的になりやすいので、これはレビュアの悪いところに間違いない。その時になるべき落ち着いて返してあげましょう。
場合によってプライベートでやり取りしても良い。
コードを修正する。コード修正で解決できるものをコード修正する。できない場合にコードにコメントを追加する、それでもできない場合にGitLabにコメントを返す。
6 Think for Yourself
Writing a CL can take a lot of work. It's often really satisfying to finally send one out for review, feel like it's done, and be pretty sure that no further work is needed. So when a reviewer comes back with comments on things that could be improved, it's easy to reflexively think the comments are wrong, the reviewer is blocking you unnecessarily, or they should just let you submit the CL. However, no matter how certain you are at this point, take a moment to step back and consider if the reviewer is providing valuable feedback that will help the codebase and Google. Your first question to yourself should always be, "Is the reviewer correct?"
開発者はコードを書いたら、やっと終わったと思いがちであり、その時もし何かコメントが来ると私を邪魔しに来た？と思うのが当然。
その時に怒らず、深呼吸してレビュアが正しいの？とじっくり考えましょう。
レビュアは常に好意を持ってコード改善しようとしてコメントしているが、やはり開発者の方がコンテキストやコードや仕様をもっと詳しいので、どうしてもレビュアが言うことが正しくと思わない場合に、素直に関連する情報を提供してコメントを返しましょう。