emahiro/b.log

日々の勉強の記録とか育児の記録とか。

トップ > Engineering > ソースコードビューについて考えていること

ソースコードビューについて考えていること

Engineering

最近、とある機会にソースコードレビューについて質問されたので、考えてること、意識していることをまとめてみました。
なお、これは私自身の考えていることで人によって異なるところは多いと思いますので一個人のお気持ち表明として読んでもらえればと思います。

目次

  • 自分がソースコードレビューのときに心掛けていること
  • テストを書くことについて
  • まとめ
  • 参照

自分がソースコードレビューのときに心掛けているいること

ソースコードレビューはレビュアーになることもレビュイーになることもあります。そのときに心掛けていることについて記載します。

レビュアーとして

  • コードレビューはバグを見つけるものではないと思ってみること
    • バグの混入はあくまで実装者の責任であって、レビュアーの責務ではない(見つけられたらラッキー程度に思っておく）
  • 極力依頼された瞬間にレビューすること
    • ソフトウェアはチームで開発することが多く、チームの全体最適を考えると極力レビューを依頼されたときにみる方がプロダクト全体で見ると速度が出ます。。これは依頼されたレビューがブロッカーになるときもあるので、ブロッカーは素早く取り除く意味でも依頼されたらまずサッと見る、というのはいいプラクティスだと思っています。
    • もちろん業務状況や勤怠状況によるので必ずこれをしないといけないという訳ではありません。
  • 「指摘」「直して」という言葉を極力使わない。
    • レビューはそもそもレビュアーとしてもレビュイーとしてもコストが高い作業なので、強い言葉は極力使わないようにしています。そのため、自分が依頼されたレビューでは以下のように言い換えるように気をつけています。
      • 指摘 -> コメント、フィードバック
      • 直す -> 取り込む

レビュイーとして

レビューを受けるときはレビュアーからのフィードバックは コードという成果物へのフィードバックであって、人格否定ではない ことを肝に銘じています。
このため 自身の成果物としてのコードに人格を載せない。成果物と人格を同一視しない というスタンスで臨んでいます。

また変更差分には

  • この変更は何か？
  • なぜこの変更をするのか？

をセットでPull Request の description に記載するようにしています。

テストを書くことについて

※ 先に言い訳しておくと、自分はテストについての専門家ではないです。

ソースコードレビューはテストコードとセットにすることが多いと思います。 テストを書くそれ自体については、色んな意見があると思いますが、自分は以下の理由からテストを書いています。

  1. デバックコストを下げること
  2. テスト = ソフトウェアの資産
  3. 「期待する動作をすること」をテストなしに説明するのがめんどくさいこと

デバックコストを下げること

実装の実践的な話になるのですが、開発するときは必ず、必要な要件に対して自分の書いたコードが正しい動作をしているかを検証しながらコードを書くことが多いと思います。
自分は普段 Go で API を書く仕事をしているのですが、実装した Endpoint に対して、

  1. 正常系と異常系のリクエストを作る
  2. local で サーバーを立ち上げる or 開発環境に撒く
  3. curl を使って動作検証を行う

という手順で検証を進めることがありますが、これがマイクロサービスでのAPI開発のケースになってくると local でも認証を突破するためのリクエスト作ったり、デバックのためにわざわざアクセストークン発行したり、といったことをする必要があります。
もちろん、最終的な検証ではこの手順を行いますが、モデルを実装しただけとか、とりあえず正常系だけ書いたので Endpoint の動作を一時的にテストしたいケースにおいてははっきり言ってめんどくさいです。

そういうときに Endpoint のテストを先に書いておいて、テストケースを追加していくだけで意図した動作になっているかはシュッと確認できるので、テストを書きます。

この開発方法を取るようになってから、開発速度が上がり始めたので、テスト書くとスピードが落ちる、は私には当てはまりませんでした。

テスト = ソフトウェアの資産

これはそのままです。この考え方を教わってからテストへの考え方が変わりました。

「期待する動作をすること」をテストなしに説明するのがめんどくさいこと

若干脳死気味でもあるんですが、PullRequest を出すときに、「なぜテストがないのか？」を説明するのがめんどくさいのでテストもセットで書いてます。
テストもなしに「自分の書いたコードは必ず正しく動きます」と文章で説明するくらいなら、テスト書いちゃった方が早いです。きっと。

まとめ

こういった考えに到るまでに様々な変更と、そのレビューを受けてきました。
何度か心が折れかけたこともありましたが、コードという成果物と私自身という人格を分けること を会得したことでかなりコードレビューというものに対してのメンタル的なコストが軽減しました。

このエントリで記載した「意識していること」のいくつかは、仕事をしている中で尊敬するエンジニアの方々からインプットされたもので僕個人の考えという訳ではありませんが、私自身としても大切にしたい考えだと思ってここに記載してます。

refs

google の文書の以下の邦訳はとてもいい文章でした。コードレビューをする・受ける側どちらでも参考になることが多いと思います。

shuuji3.github.io