Upgrade to Pro — share decks privately, control downloads, hide ads and more …

コードレビュー研修

Yuki Toyoda
July 21, 2020
9.9k

 コードレビュー研修

2020/07/21 に弊社新卒向けに実施したコードレビュー研修の資料です。

Yuki Toyoda

July 21, 2020
Tweet

Transcript

  1. この研修では • Brand-new 2020 に向けた研修資料です。 • 何を話すべきかよくわからなかったので、レビューに際して 私が気をつけていることを書きました。 ◦ わたしはこういったエモ寄りの話はほとんどしないので、貴重な機会です(?)

    • 開発プロセスの中にレビューの実施がすでに盛り込まれていることを前提としています。 ◦ なので、そもそもチームにどうレビューを根付かせるかという話は含まれていません。
  2. チェックポイント(前提) • レビューをし合う。みんなでやる。 ◦ 詳しい人の承認をもらう形式にするとちょっと安心できるけど、基本はみんなでやる。 • すべてをチェックはしきれない。 ◦ レビューしなければまずい箇所を見逃すことはある。 ◦

    不具合の発見はユニットテストなどでも行う。 • レビューが正しく機能するためにはそもそも 仕様がきっちり決まっていないと無理。 ◦ 仕様がないor文章化されておらず共通認識が取れていない状態でレビューをしても、本当に果たしたかったレ ビューの目的は果たせない。 • レビューを通じてディスカッションをする。 ◦ 客観的な視点を取り込みながらコードはよくなっていくというのを忘れない。
  3. チェックポイント 下記は私が個人的に見ているところ。 • 明らかに仕様と違う実装がされていないか? • コードの統一感はとれているか? ◦ キャメルケース・スネークケース。関数名が適切か。フォーマッタはあたっているか。 ◦ リンターに任せてしまうプロダクトも多いかも。

    • ファイルの配置場所、パッケージの構成は合理的か? ◦ アーキテクチャの意図する場所に置かれているかを見ます。 • パフォーマンス上明らかにまずそうなものはないか? ◦ N+1 をやっていそう、計算量が明らかに多い、など。
  4. コメントを書く際の心構え • コードレビューはコードの欠陥を指摘する行為ではない 。 • したがって、欠陥を指摘しようと意気込まない 。 • レビューは誰もが平等にするし、またレビューを受ける必要がある。 •

    よりよいソフトウェアにするために 議論する場と考える。 • 自身の主張の正当性を、レビューコメントの辛辣さや皮肉で主張しない 。我々エンジニアは科学的手法と 理性による判断の正しさを信条として生きるべき職種である。あくまでロジカルに。
  5. • 「ここはオブジェクト指向ではありませんね」「関数型らしく書いてください」「 DDD によると XXX (XXX は DDD の用語) ではない」のような、ある種の専門用語をふりかざす

    だけのコメントをしない。 • 「データの内容は、一般的にはひとつクラスや構造体を用意して、その中で扱うようにします。こうすると、 バラバラだったデータの内容に対して我々が普段業務で使う言葉で命名することができ、より可読性が上 がり、プログラマ間での意思統一がはかりやすくなると思います。具体的には、下記のコードのように直し ます。」など。 • 具体的にどうしてほしくて、結果何が嬉しいのかを詳細に書く。 コードを交える。 • どうしても固有の用語を使用したい場合は、レビューを受ける人の理解度に応じて参考リンクを貼るなど する。 衒学的なコメントをせず、より具体的に
  6. • 主張には客観性がなければならない。 • 客観性の担保には、下記が使える。 ◦ ネットの記事で構わないので、参考情報を付与する。 ◦ 公式サイトやドキュメントからの引用をする。 ◦ 先行事例や参考情報、手法(プロダクト内のものでも可)が他にあるのであれば出す。

    ◦ 他にもあるかな? • 自身の主張について第三者も意見して 反駁できる状態にしておく のが理想。そのために論拠やソースが 必要。論拠やソースを辿って思考手順が正しいかを見たいから。これを反証可能性と呼ぶが、科学では もっとも重要な営みの一つ。 主張の客観性を担保する
  7. よいプルリクエストを作るには 私がレビューしやすいなと感じる PR に共通するポイントは下記。 • PR の概要が書かれている。 • 差分(あるいは PR

    )の目的が1つに定まっている。 • 差分を大きくしすぎない。かといって細かくすればいいというものでもない。 • 重点的に見てほしいポイントが書いてある。
  8. PR の概要が書かれている 当たり前かと思うかも知れないが、必要十分な概要になっていないことは多い。 下記を書いてもらえると、レビューしやすい。 • PR の背景や目的を書く。 ◦ 多くの場合、解決したい問題、追加したい機能が書かれるはず。 •

    検証内容を書き、その結果を貼る。 ◦ テストを手動で実施した場合はその内容。 ◦ ユニットテストの概要。 ◦ 画面変更の場合はスクショだとわかりやすい。 • 次のタスクとして何が想定されるかを書く。 ◦ そのコードの評価は他の機能との関係性によって決まるので。
  9. 差分を適切なサイズに保つ • これは難しいが、よく「一つの PRのdiffを小さくしよう」というベストプラクティスのようなものが言われる。 • かといって、関数ひとつだけ実装して、とくに動きもしないみたいな PR はしんどい。 • 「ひとまとまりに」なっていたほうが、むしろ嬉しい。

    • ただ、その「ひとまとまり」が大きくなりすぎると辛い。見逃す。 • 大きくなった場合には理由を書く。自動生成などは大きくなりがちで仕方ない場合もある。 • 差分が大きくなりすぎる場合は、だいたいの場合はタスクの切り方に問題がある。チームでタスクの切り 方を見直す必要がある。
  10. 参考文献 • アリストテレース『詩学』(岩波文庫) • 『日本文化の核心』(講談社新書)松岡正剛 • SECI モデル: https://mba.globis.ac.jp/about_mba/glossary/detail-11667.html •

    反証可能性: https://www.cscd.osaka-u.ac.jp/user/rosaldo/080428falsifiability.html • PREP 法: https://webtan.impress.co.jp/e/2017/06/08/25694 • Code Health: Respectful Reviews == Useful Reviews: https://testing.googleblog.com/2019/11/code-health-respectful-reviews-useful.html • Why review code?: https://sophiebits.com/2018/12/25/why-review-code.html