「一人でもコードレビューはできる！」Sendagaya.rbでコードレビューについてディスカッションしました

先日デキるプログラマだけが知っているコードレビュー7つの秘訣というテーマでSonicGarden Studyというオンライン放送にてお話をさせていただきました。参加者が500人とかなり大盛況の放送だったのですが、その分流れる質問量も激しく、十分に質問にお答えできなかったなーという感がありました。そこでその放送の翌週にあたる8/18（月）にSendagaya.rbの場をお借りして（thanks @tkawa！）コードレビューについてディスカッションする機会を設けさせていただいたのでした＠ソニックガーデンオフィスにて。

スライド

前半はこのスライドを元にざっくり15分ほどでSonicGarden Studyでどんなお話をさせていただいたかを紹介しました。3分ぐらいでざっくり、と前置きしたのに15分ぐらい話してた・・・。

デキるプログラマだけが知っているコードレビュー7つの秘訣 from Masahiro Nishimi

話に上がった内容

いろいろと面白い話が上がっていたので、覚えている範囲でつらつらと箇条書きしてみます。

一人でコードを書いているので、一人でプルリクエストを送って、他人になった気持ちで一人でコードレビューをしている。コードレビューは必ずしもチームでなくてもできる。
インフラ周りのコード（Chefレシピなど）を書いているが、他のメンバーはアプリ担当なので、インフラ周りのコードをレビューできる人がおらず、何かあったときに不安。
チームメンバーのうち一人のレベルが高すぎて、その人のコードはレビューできている気がしない。
実力の高い人に見てもらうと「あ、そうなんだ」という気付きがある。
チケット単位にプルリクエストしている場合、そのチケットが大きすぎると「どこを見れば良いの？？？」という状態になる（Addition/Deletion共に5000行overとか）。
- 小口化しないと回らない。
厳しいプロジェクトだとレビュアーがプルリクエストされた機能の動作確認をする文化がある。でもそれってコードレビューの範囲内なの？
コードレビューでは設計に対してのレビューはできないのではないか。そもそもコードの品質は設計レベルに問題があることが多い。設計に対するレビューはどのように行っているのか？
コードレビュー時の設計流派の対立はどう解消するか？　例えば複数モデルにまたがるトランザクションを処理する際にサービス層を取り入れるか、取り入れないか、など。
- 対立と言うか、テスタビリティの良い方に倒せば良いのでは。そういった意味ではテストがないと新しい設計にトライすることも難しい。
テストが全くない場合はどこからテストを書けば良い？
- featureテストから書くのが一番費用対効果が良い。
プロマネとしてスケジュールを引くとき、コードレビューの時間はスケジュールに入れて考えているか？
- コードレビューの効果をきっちり説明できるならスケジュールにも入れることができる。
コードレビューをしっかりしていると、マネジメント層から見て開発の進捗が遅く見えるので「コードレビューするな！」と言われる。つらい。
- そもそも何のためにコードレビューをしているのか共有できていないと、つらくなる。
スピード＆値段勝負で受託開発をしていると、コードレビュー分の時間が無駄に見える。「うちはテスト書かないからもっと安くできます！」というベンダーすらいる。
- コードレビューは保守性を高めるためのアクションなので、そもそも保守性が問題にならないようなビジネスだと効果が出づらい。
テストがないと仕様が分からないのでコードレビューしようがない。
コードレビューの場は仕様の共有の場でもある。お互いのコードを知らないと、何かトラブルがあったときに対応できなくなる。
コードレビューは「引き継ぎ」の小口化とも言えるのでは？
仕様レベルの確認のために、featureテストだけコードレビューしてもらう、という試みもアリなのでは。
コードレビューは自然と表現がキツくなってしまうことがあるけれども、コメント文のレベルで何かできる工夫はある？
- emojiを多用すると多少雰囲気良くなる。