Zenn
👓

コードレビューが早く終わる理由、遅れる理由、悩む理由

2024/10/25に公開
チーム開発
チーム
コードレビュー
tech

はじめに

ここ1年半くらいで、

  • コードレビューが日の大半〜1日を占めることも少なくないエンジニアリード層（？）
  • コードレビュー専門の担当

を経験して、レビュアーとして思った気づき。

レビューの進捗度合いは様々であり、思い通りに行かないことも多い。
それを分解して洗い出すと次のような感じになる。

実装のボリューム

レビュアーが楽をしたい怠け者なら、量が少ないほど取っ掛かりやすく見える。
とはいえ単純にコード量が少ないと楽という訳ではないので、この考えは痛い目を見る確率が高い。

実装の領域

極端な話、フロントエンドならマークアップ・データフェッチ・フォームロジック・グローバルな状態、バックエンドならMVCやクリーンアーキテクチャの全レイヤーをデカ盛りで出されると苦虫を噛み潰したような表情になってしまう。

もっともこれは自分自身もある程度の規模を経験して初めて身に付いたので、タイミングがないと自覚するのはなかなか難しいかもしれない。

そしてこの場合はレビュイーも大量の作業をやりきった達成感に満ちていることが多い。（誰もが通る道）

重要度

定型的なタスクなら楽だが、開発の根幹に関わる実装の一手である場合はそれなりに心の準備が必要。
いわゆるはじめての◯◯。

レビュアーもレビュイーも深く準備しておくスキルが求められる。

ヨシ！の精神

思考停止に陥る。

  • なんか面倒くさい
  • なんかレビュイーに悪いと思うから甘めでいいや
  • 時間がないから甘くする
  • 何となく信頼できないから辛口評価。私は細かいのがウリだからこれでいい

PR作成のスキル

  • PRの背景を説明する
  • PRに基づくチケットリンクやスクショを貼る
  • 連続する作業のPRを作る場合は、ブランチを次々と分岐させる

など、これらの一例のようにPRを配慮することによりレビュアーの心理的負担が減る。
コードと同じくPRも書くより読まれる方が多い！って近所の猫が言ってました。

優先度アルゴリズム

レビュアーは様々な優先度をこねくり回して着手している。

  • 基本はReview Requestsの日付が古い順でいいだろう。しかし...
  • 定型作業でやっつけられそうなものは早く片付けたい
  • レビュイーが初心者で迷ってそうなものは早く片付けたい
  • 今後の方針に重要な影響を及ぼしそうなものは早く片付けたい
  • 何となく優先的と言われている機能のものは早く片付けたい
  • あのPRはこのPRの後に見たい
  • 複数のPRが関連性を持っている場合はそれらを塊で片付けたい
  • 簡単そうだと思って取り掛かったら思いの外予想が外れた
  • イレギュラーな差し込み作業
  • etc...

好感度

これを言ってしまうのは心地が悪いが、作業の進捗はレビュイーの好感度にも左右される面があると思う。
以前の大規模現場にて自分含めてそういう現象はちらほら発生していたことが分かった。そしてその気付きからより意識的に自覚し、自分自身も偏見を正して公平にレビューしようと心がけるようになった。

好感度と冒頭で書いたが、具体的は「非協力的に見える」ということに尽きると思う。
例えば、

  • レビュアーとのコミュニケーションの精度があまり高くない
  • 単純にスキルが低い
  • 分かりづらいPRを書く
  • PRの管理がなおざりな状態になりがち
  • 単にあまりコミュニケーションしてない

のようなケースがよく起こりがち。

どう伝えるか

レビューのコメントでどのように伝えるかの温度感。
例えば初心者には丁寧に、エキスパートにはポイントを抑えてサッとというのが丁度良いと思われる。

レビュイーの度量はコードを一読すれば何となく分かるが、読み取りづらいときもある。そして上記の温度感が逆転してしまえば、ズレちゃうな〜と危惧する。

効率化を追い求める努力

レビューの指摘がプロジェクトで何度も頻発するようなら、レビュー指針やレビューのチェックリスト、また実装方針の周知や勉強会も考えなければならない。それを俯瞰するのはレビュアーの役目。

余談ですがコメントにshouldとかmustとかnitsとか付けるのは一時期好んでましたが、次第に合わない気がして今は辞めました。

コーディング

常に1から10まで完璧に動作検証する訳ではないが、レビュワーがレビュイー以上にコードを書いて多大な検証や考察に時間を費やすことも多い。
実装してみて初めてレビュイーの穴や、むしろ自分が指摘すべきと誤解していた誤りが見えてくる。

上記作業をサボると単なるコードに文句つける係になってしまう。

レビュイーとの温度感のズレ

ここまでやってOKという期待値にズレがあるケース。
例えばFEエンジニアとして感じる頻発例は...

  • デザインと多少...でなくもっと普通にずれていてもOK

    • 例えばSpacingが10〜20pxくらい違う
    • Web制作系、システム開発系の経験者のどちらともかなり多そうという感覚がある

  • TypeScript型定義が甘い、むしろ型付けを嫌がるような書き方

    • 動的言語の経験者に多いと思われる。（自分も最初は正直苦戦したので慣れでしかない...）

というのが巨頭？だと感じる。

もちろん3年近くFE分業体制のぬるま湯で暮らし続けている自分が気晴らしにBEのコードを書いたら袋叩きにされるレベルでしょう。なのでいかなる温度差も許し、受け入れ、想定しましょう。

終わりに

コードレビューは改めて考えると今までに書いた煮凝りを凝縮したようなもの...ですが、ある意味自分で一から考えるプレッシャーは薄めなので楽といえば楽な作業だとも思います。

コーディングもコードレビューも抜かりなく行って、チームで良い関係を築いて行きたいですね。

Discussion