コードレビューの思想や心構え

コードレビュー

Last updated at 2023-12-04Posted at 2023-12-01

株式会社ブレインパッドでデータサイエンティストをしているasanoです。

この記事はBrainPad Advent Calender 2023 1日目の記事シリーズ2です。
※シリーズ1は@fuyu_quantさんの入力プロンプトを復元する技術 #ChatGPTです！

今日はコードレビューの思想や心構えについて書きます。

はじめに

コードレビューをより生産的に進めるには単にコーディングのスキルだけでなく、そもそものコードレビューに対する思想や心構えについても一定のリテラシーを求められると考えています。

コードレビューはどうしてもロジカルな話になるため伝え方にも気を付けないとモチベーションの低下に繋がりやすいと考えています。

そうなると当然パフォーマンスも下がってしまいます。

これを防ぐために自分は「コードレビューの思想・心構え」をまとめてチームのガイドラインとして使っています。

あくまで主観でまとめたものであり、コードレビューに絶対的な正解はないと思っておりますが、チームの認識合わせに使うには十分だと考えております。

以下2つを区別せず書いています

対面での実施やオンラインMTGツールなどを使った口頭での同期レビュー
プルリクなどテキストでの非同期レビュー

また、本ドキュメントではコードに関する具体的なテクニックについては触れません。

コードレビューの目的

チーム全体のスキルを上げる
コードの品質を保つ
- 他人に見てもらうことで汚いコードが放置されることを防ぐ
- （参考）割れ窓理論：建物の窓が壊れているのを放置すると、誰も注意を払っていないという象徴になり、やがて他の窓もまもなく全て壊される
- 岡目八目だから他人が見るとすぐに気づくことがある
- 書いた本人も壁打ちすることで気づくことがある
効率化
- コードレビューをすることで綺麗なコードになる → 後からコードを読む時間が短くなる
- つまりコードレビューは投資の時間

用語

コードレビューに参加する方に最低限覚えてほしい用語です。

レビュアー：レビューする人
レビュイー：レビューされる人（コードを書いた人）
リファクタリング：外部から見た時の挙動は変えずに、内部構造を整理すること。機能追加やバグ修正はリファクタリングではありません
DRY原則：「Don’t Repeat Yourself 繰り返すな」　同じ意味や機能を持つ情報は1カ所にまとめましょう
KISSの原則：「Keep It Simple, Stupid. シンプルにしておけ、愚か者」「Keep It Short and Simple 簡潔かつ単純にしておけ」コードを書くとき、最優先の価値を「単純性」「簡潔性」に置きます

参考

思想

コードスメルに気をつけます（コードスメル：コードのあるべき状態でないこと全般）
細かい美しさよりも大局的な美しさを大事にします
人間のレビューは貴重なので機械ができることは機械に任せましょう
スタイル等はlinterやformatterに任せましょう
- Python
  - Black：https://black.readthedocs.io/en/stable/
  - autopep8：https://github.com/hhatto/autopep8
- SQL
  - SQL formatter：https://marketplace.visualstudio.com/items?itemName=arniu.sql-formatter-vscode
  - JOINとONが同じインデントになるのが個人的には使いずらいですが、何個かSQLのformatterを使った中では一番よかったです
背伸びしない範囲でのリファクタリングに取り組みます
チームで決めたルールを明文化します
世の中に完璧なコードはなく、より良いコードがあるだけです
- 解はグラデーションです
- 0 or 1のどちらかが正しいわけではなく、状況に応じてバランスを考えたより良い解を選びましょう（最適解を選ぶ必要はないです）

参考

心構え

レビュイー、レビュアー共通

レビューする、されるのは成果物
コードに厳しく、人に優しく

レビュイー

レビュアーに頼りすぎないこと。
あなたが書いたコードにはあなたが責任を持ちましょう
（とはいえチームみんなで協力していきましょう）
レビューしてもらうコードの整合性やロジックは合っているのが大前提
少なくともあなたができる範囲で最低限の確認はしましょう
レビューしてもらいたい観点を明確にしておきましょう
適度な量のプルリクにしましょう
- 500行で1時間かかるイメージ
  これを超えると不具合を見つけにくい、集中力も続かない、費用対効果に合わない。途中からあら探しになるだけ。

参考

【IMO】コードレビューって難しいよね.pdf - Speaker Deck

レビュアー

良いコードは良いとコメントしましょう
自分では思いつかなかったコードや前回指摘した点が直っていたら「良い！」と言いましょう
私もまず良いことを1つ以上コメントしてから改善点を伝えるように心がけています
　　
優しい言いまわしを心がけましょう
どうしてもロジックの話になるのでそのつもりがなくてもレビュアーが必要以上に落ち込むことがあります。
テキストベースだと特に冷たくなります、絵文字を使うと柔らかい印象になるのでオススメです😉（参考：emoji-cheat-sheet/README.md at master · ikatyang/emoji-cheat-sheet · GitHub)
　　
コメントする際は優先度やお気持ちを示しましょう
テキストベースであればprefixをつけましょう
- MUST：絶対直してください（ロジックが正しくないなど不具合が起きている、起きそうなとき）
- IMO：（In My Opinion）私はこう思う
- IMHO：（In My Humble Opinion）私の控えめな意見としては...こうだと思います（謙虚な主張）
- nits, nitpick：あら探しですが...（スタイルの話、pep8に合っていない、どっちでもいいけどこの変数名のこの単語はこっちのほうがよいかもねーとか）
- ASK：純粋な疑問、質問（レビュイーが変に勘ぐってしまうのを防ぐ）
- LGTM：（Looks Good To Me）良いと思います！（完璧ではないけどマージしちゃっていいんじゃない？のレベルならLGTM）　
  　　
指摘、コメントの際は解決策だけでなく「なぜそうするか」の理由も説明しましょう。
- 時にはレビュイーの成長に繋げるため理由や問題点だけ説明し、レビュイーに具体的な解決策を考えさせます
  　
一回のレビューで終わらせましょう
- IMOやnitpickが直っていないときはほかのタスクとの兼ね合いもあるので「次のコードから直していきましょう」と暖かく見守りましょう
- ただしMUSTは直してもらいましょう。なぜならMUSTだから。　
  　　
些細な指摘(IMO/nitpick)は5つ以内にしましょう
本当に修正してほしい指摘を埋もれさせないためです
- ※ Googleに学ぶコードレビューのポイントには「レビュアーは何か改善できそうな点を見つけたら、コメントを残すのに躊躇する必要はありません。」とありますが優先度が低いnitpickなコメントは数を絞ったほうがよいと思っています。
  ただこれは私自身が普段アドホックなデータ分析をする場面が多く、プロダクト開発をしているわけではないというのがこの思想に反映されていると思います。

参考

結局

本質に向き合う
敬意を払う

が大事

参考：株式会社ブレインパッド VALUES

118

104

You get articles that match your needs
You can efficiently read back useful information
You can use dark theme

What you can do with signing up