engineering-review-comments

エンジニアリングレビューのコメント

レビューコメントは、コードと開発者の今後の判断力を向上させるものであるべきです。重大度を明確にし、エンジニアリング上の根拠に基づき、個人ではなくコードに向けた内容にしてください。

Google Engineering Practices Documentation、特に「コードレビューコメントの書き方」と「コードレビューで異議が出た場合の対応」を参考にしています。出典：https://google.github.io/eng-practices/review/reviewer/comments.html。ライセンス：CC-BY 3.0。

コメントの形式

重要なコメントには、次の形式を使用してください。

SEVERITY: Observation about the code.

Why this matters: engineering reason, user impact, maintainability impact, or violated project rule.

Requested action: concrete fix, question, or decision the author needs to make.

短いコメントは短くまとめてください。意図が誤解される可能性がある場合は、完全な形式を使用してください。

重大度ラベル

• BLOCKER：正確性、保守性、セキュリティ、データ、テスト、または運用上のリスクが許容できないため、承認前に必ず変更が必要です。
• IMPORTANT：承認前に変更するべきです。明確な根拠がある場合にのみ、変更を見送れます。
• QUESTION：判断する前に、レビュー担当者が情報を必要としています。
• NIT：軽微な修正です。承認を妨げないでください。
• OPTIONAL または CONSIDER：作成者が採用しなくてもよい提案です。
• FYI：教育的な説明、または将来の検討事項です。対応は不要です。

書き方のルール

• 作成者の人物評ではなく、コード、設計、テスト、または動作についてコメントしてください。
• 明らかなスタイル上の修正や機械的な修正以外を求める場合は、理由を説明してください。
• 「X が難しくなるため、こうするとよいです」のように書き、「X は好みではありません」のような表現は避けてください。
• 通常の動作を理解するのにレビューのスレッドでの説明が必要になる場合は、コードをよりシンプルにするよう依頼してください。
• コードだけでは理由を合理的に表現できない場合に限り、コードコメントを追加してください。
• 作成者に実装を推測させるよりも、直接的なコード例や具体的な設計を示すほうが迅速または安全な場合に限り、それらを提示してください。
• それ以外の場合は問題を説明し、実装方法は作成者に委ねてください。
• 役立つ場合は、優れたエンジニアリング上の判断も指摘し、何が良いのかを説明してください。

表現のパターン

必要に応じて、次のパターンをそのまま使用してください。

BLOCKER: This path can return success before the write is durable.

If the process crashes after this point, callers will believe the operation completed even though the data may be missing. Please either move the success response after the durable write or make the operation explicitly asynchronous and observable.

QUESTION: What guarantees that this callback cannot run concurrently?

If it can run in parallel, `pending` can be decremented twice from the same value. If the framework serializes this callback, please add a short comment at the boundary where that guarantee comes from.

NIT: This helper name reads as if it validates the payload, but it only normalizes whitespace. Could we rename it to `normalizePayloadText`?

OPTIONAL: Consider extracting this branch into a named helper. The current version is correct, but a helper would make the three policy cases easier to scan.

異議が出た場合の対応

作成者が同意しない場合：

1. まず、相手の主張を再検討してください。相手のほうがコードに詳しい、または見落としている背景情報を把握している可能性があります。
2. 相手が正しい場合は、率直に認めてコメントを解決してください。
3. それでも意見が異なる場合は、相手の主張を言い換えてから、依頼する理由となるコードの健全性上の懸念を説明してください。
4. 必須の依頼と好みを区別してください。
5. スレッドで進展がなくなった場合は、より情報量の多い議論の場に移し、決定事項をレビューに要約して戻してください。

最初にコメントを書いたのが自分だからといって、議論を続けないでください。目標は最善のコードであり、最初の表現を擁護することではありません。

よくある間違い

• ラベルのないコメントを残し、作成者に必須の指摘だと受け取られる。
• 指摘自体は正しくても、協力しにくくなる口調で書く。
• 分かりにくいコードについて、レビューのツール上だけで説明を求める。
• すべてのコメントを、設計全体を任せる依頼にしてしまう。
• 教育的な注記をブロッカーとして扱う。