git-reviewer 書いた
code review の reviewer 選出をする時,pull request の内容をざっと眺めてから「この部分だから XX さんかな」とか「あそこのコードは YY さんが詳しいだろう」とか,そういう感じで選ぶことが多くて,つまりは勘と経験で選びがちになってしまう.これについては常々いくばくかの危うさを感じていた.
そもそも,「reviewer として誰が最適か」という知識はプロジェクトに長く関わっている人でなければ知りにくいものであり,いわば属人的な知識のひとつだと思っている.プロジェクトからそういった長老的な人がいなくなってしまったら,最適な code review を実施できなくなってしまう可能性がある.
従って,やはり技術で解決ということになる.
Facebook が作っている mention-bot という GitHub の bot として動作するやつがあって,これは pull request が送られてくるとその pull request について blame を実行して code reviewer の候補を出してくれるという気の利いた処理を自動で行ってくれる.
mention-bot は便利で,我々も使っているのだけれど若干の不満もある (おおよそ良いのだけれど……)
- WIP の pull request だと,作業途中時点での reviewer が選出されてしまう.mention-bot は pull request が送られた時点での reviewer を選出してしまうので,WIP pull request との相性が悪い.
- GitHub じゃないと動かない
後者は身も蓋もない話だけれど,前者については若干問題があるなーと思っていた.
任意の時点での reviewer 選出をもっと気軽にできれば良いのに,と.
というわけで,git-reviewer というスクリプトを書いた.手元で git のサブコマンドを実行することで reviewer 候補の選出が出来る.
使い方は至って簡単で,
$ git reviewer <into branch> <from branch>
としてやると,その brnach 間の diff について最適と思われる reviewer を選出してくれる (into branch を省略すると,current branch が into branch として扱われる).
仕組みとしては極めてシンプルで,
1. branch 間の diff を取る
2. diff が出た各ファイルについて,削除された行をかき集める (すなわち,diff の先頭に - が付いている行)
3. 削除された行 (つまり変更を入れられた行) のもともとの author を git blame により特定する
4. その author をかき集める
という処理を行い,頻出する author が reviewer 候補として選出される.
もしも diff に削除行が1つもない場合は,変更があったファイルの全行についてその author を集計し,その数が多い人を reviewer 候補として扱うようにしている.
このコマンドを手元で実行することで,手軽に reviewer 候補を知ることが出来て便利になった.めでたしめでたし.
何か「こうした方が良いのではないか」「おかしいのではないか」などがあったら教えて下さい.
[追記]
実行してみればお分かり頂けると思うのだけれど,git-reviewer の出力はとてもシンプルなものになっている.以下のような感じ.
moznion: 123 nozniom: 42 foobar: 2
これらは影響行数の降順として出力されるので,上に表示されればされるほど reviewer 力が高いという事になる.
もしも除外したい committer がいるならば,パイプで grep -v
とかで除外すれば良いのかな〜とか思っていたのだけれど,確かに reviewer 側のオプションで食わせられるようにしても良いかもしれない.これが UNIX 哲学だ!! と頭ごなしに殴りつけても良いことはないのです.参考になりました.
以下はコミュニケーションの様子
コードレビュアーの選定は多くの場合経験と勘によって行われていて、これは言語化されていない知識によるものなので、レビュアーの選定というのは皆さんの嫌う属人的な作業になっている場合が多い。当たり前のように行っているあまり、それに気付かないことも多いのだけど
— 病弱 (@moznion) 2016年5月25日
一方でコードレビューによる属人性の排除という視点もあり,そういったばあいはおみくじが有効ということになっていたはず
— 浸食_〜lose_control〜 (@side_tana) 2016年5月25日
コードレビューで属人性が排除できるの夢では,といった指摘については別のところでやったほうがよさそう
— 浸食_〜lose_control〜 (@side_tana) 2016年5月25日
とはいえ、いきなりなにも分からない人にレビュアー依頼が飛んでもお互い不幸なので、現実的な落とし所としてこれかな〜って感じ!!
— 病弱 (@moznion) 2016年5月25日
実装俺、レビュアー俺なのでどうにも。
— Atom(アトム) (@FromAtom) 2016年5月25日
@FromAtom 選手兼監督っぽい
— 病弱 (@moznion) 2016年5月25日
@moznion ヤクルト古田の苦労が忍ばれる。
— Atom(アトム) (@FromAtom) 2016年5月25日
@FromAtom ガッハッハ (ガッハッハではない
— 病弱 (@moznion) 2016年5月25日