「コードレビューをキメると品質も上がるし自分のレベルも上がるので最高」みたいな論が巷を賑わせていて、以前はそういうイケてる制度を指をくわえてみるのみだったのだけれど、最近職場と、それと個人的に関わったプロジェクトでコードレビュー制を無理矢理交渉して導入してみた結果、世間のイケてる書籍やエントリから得られる情報とはまた少し違う知見が得られたので書いてみる。
割と泥臭かったり、あまり希望に溢れてたりはしない感じのエントリなのでそういうのは期待しないほうがいいです。
準備
些末なコードレビューを極力避けるために、コードの規約やスタイルについてはlintとフォーマッターを用意した。
他は無策。
結論
結論から言うと、理想的な運用は出来なかったものの、コードレビューについて世間で言われるような成果(作業を共有する意識、レベルの向上)は得られた。良かった。
ぶっちゃけ僕なんかが浅はかな考えで導入してもいい感じの成果が得られたのってすごいことなんじゃないかなと。
というか、レビューの効果云々以上に「自分のコードが他人に読まれる」という意識を持っていなかった人が、それを持つようになるだけで書くコードに雲泥の差が現れたのに驚いた。本当に、半月でビックリするくらい書き方が変わるのが見ていて面白かった。
その一方で、あまりこういうことは書きたくないけども、そもそも伸びしろがあまりない人だとそういう効果は現れないし、'向上心' or '好奇心'に欠ける人間だと「なんか五月蝿いこと色々言われて全然仕事が進まないんスけど」みたいな感じで、モチベーションをダダ下げするという悪い? 効果を生み出して雰囲気が最悪な感じになったりしてヤバかった(ここら辺は僕の性格の悪さとかも影響してると思うので、もっとコミュ力とか権威のある人間がやれば違う結果が出ることはありそう)。
ただし、コードレビューというと、チーム全体の技術力向上! みたいなものがよく取り上げられると思うけど、これは半分しか正解じゃなくて、コードレビューで得られるのはどちらかというと即物的な知見がメインでレビュイーの地力以上の成果は出にくいと思った。
思想的なもの、パラダイムの違い、などの高度なトピックスだとか、そういったものをコードレビューだけでまかなおうとすると死人が出る。
そして、なんとなくモヤモヤするようなそんな成果であっても、それを得るための道のりは平坦では当然なくて。
ちょっとの風向きでチームが崩壊してた可能性はあるし、支払ったコスト(主に僕の時間的、精神的コスト)は決して安くなかった。正直に言って、各々が自然にコードレビューをし合うような性質のエンジニアの集まりでもない限り、コードレビューの実施は苦労とリスクが高いのでは? というような印象。
理由については以降で述べようとおもう。
実施前の状態
たぶん世間一般的な開発チームって感じ。
協業よりも分業で「ハァ? あそこはあいつの書いたコードだから俺はしらねーし責任ねーし触りたくもねー」という、基本そういう雰囲気(盛ってます)。開発言語(Python)についても、詳しい人はおらず、その他のノウハウの共有も皆無で誰か一人がトラックに轢かれるとそこで一旦業務がストップする感じ。
まぁまずここでギャップがあって、マジうまく行ったぜ! 記事は、基本的に
- プライベートな時間を使って高いレベルの技術記事を書く人(技術力 + ソンケイ)
- コードレビューの実施などに理解のある組織
- 所属エンジニアの平均レベル高い(っぽい)
みたいな感じで、そもそもスタートラインが違う感じがする。
「エースみたいな人以外のエンジニアは割と平均レベルですよ〜」みたいなのもそもそもが謙遜盛りの話であって、コードレビューが文化として定着するような組織 && そういう組織に参加出来る人間の学習意欲や適応力が低いわけないだろう! というお話。
「些末なコードレビューやめろ!」論にしても、それってそもそもチーム全員が些末なレビューを必要としない技量であることが前提なんですよね。
導入するまでの経緯
うちはそもそも少人数のチームではあるんだけども、それでトラックナンバーが1だというのはマジで危機的だと思ったので、転職して2〜3ヶ月後にはコードレビューの必要性を説きまくった。
割と放任な雰囲気があるおかげで、「やりたければやれば? 意味あんのかしらんけど」みたいな感じの承認は得られたのでとりあえず実施することにした感じである。
また、もっと偉いほうの人には「やりたいならやってみればいいけど、そういう他人に口出しするようなことしてないでお前が3倍仕事進めれば済む話だろ」みたいな忠告を頂いた。
順風とは行かなくても、少なくとも導入に際しての逆風は吹かなかったあたりで、僕は結構恵まれてる気もする。
ただし、3倍の話もある通りに、ほぼ僕が勝手にやってるようなもの(お手伝いプロジェクトの方は組織的な理解があった)だったため、レビューによる作業の遅滞はあまり許されない感があったので精神的にかなりクるものはあったし、レビューが雑になることもあった。
マッチョ的に思考すると、結果的に成長のカンフル剤になったので良かったとも言える。
フリーダムなチームに属してる僕でそうなので、社員が余計なことをするのを嫌う組織にいる人はもっともっと苦労することを覚悟したほうが良いと思う。
(追記)
気がついたらブクマ増えてて、「お前が3倍〜〜」のくだりにいくつかコメントがついていたので追記。
この話は 【中編】華麗なるキャリアの道程は、『ドワンゴ』から逃げ出したい一心から!? / 飲み会で探るエンジニアのホンネ #naoya_sushi 編 のスクラムのくだりともしかしたら似ていて、「みんなの平均点をあげたところで、新製品開発のような並列的作業が困難な創造的フェーズにはほとんど貢献しないぞ」というようなニュアンスだったっぽいです。ちなみにその偉い人は名実ともに偉い感じで、僕が今の会社にいるのもその人がいるからということでかなり尊敬している感じの方なんですね。言うことはいちいち過剰ですが。
ちなみに僕は、そういう初期段階においても信頼して一緒にやってける強いチームが作れたらなぁという意図があったので、どっちにしろそこで対立はするのですが。
(end 追記)
うまくいったところ
プログラミング暦が長い人、または適応力の高い人、僕と性格的な相性が良かった人(どうしても僕が中心だったため)とのレビューは、指摘へのレスポンスの良さだったりとか、後で全体のドキュメントに残したくなるレベルの議論に発展したりだとか、すごい有意義な時間になることが多かった。世間がこれほどコードレビューを持ち上げる理由もわからなくもないなという感じだった。
あと、海外の企業が、採用にあたっては絶対に所属予定の開発チームとの相性面接をやるっていうのもきっとこういうところにあるんだとおもった。
入社して日が浅い時から無理矢理続けていたため、自然と僕が読むコード量も増えて、チームの作業にコミットできるようになるまでの時間も短縮出来た。
これについてはペアプロをやれば一層の効果をあげられたと思うのだろうけど、とりあえずレビュアーを買って出るだけで全然違うと思った。
また、当初の目論みであったコードの共有というか、共犯意識みたいなものを作り出すのも70%くらいは達成出来た感じがあったので良さがあった。
「今まではよくわからないまま闇雲にコーディングしてたけど、レビューのおかげでなんとなく指針みたいなものが掴めてありがたかったです」みたいな意見をもらった時は、無理矢理に導入して本当に良かったなと思いました。
どうしてもレビュワーとしての感想が多くなっちゃうけど、僕がレビュイーになることもあって、特に雑なコードでバグを作り込んでる時のレビューはかなりありがたかった。
あと、業務では使ってないけどGitHubはマジで便利だとおもった。プルリクエスト -> レビュー -> マージの流れはすごくいい。いいっていうのは聞いてたけど実際にやるとまじで良かった。
うまくいけなかったところ
全員をレビュワーには出来なかった。
あまり自分の意見を表に出したくない人、作業だけこなして給料だけもらっていたい人もレビューに巻き込むことは出来なかった。
あと、当然のように自分の能力以上のレビューは不可能。そういう点で、それこそ別のチームも含めた他人をもっと巻き込んで、色んな視点でのレビューを見れたらもっと面白かったのかなーとおもった。
ヤバかったところ
僕にしても、ただの思い込みや見切り発車で導入したわけではない。なので、以下にあげる
ボリュームが多くなるので一つづつあげる
自分の技量に自信が持てない状態で始めた
有り体に言って、当時プログラミング歴3年くらいしかない奴があれこれと口突っ込むのは、想像以上にしんどかった。
これが体育会系だったら、調子のってんじゃねえぞって干されても無理ない。最悪校舎裏に呼び出されて火のついたタバコを押し付けられたりとかするやつ。
「このコードヤバいよなぁ」って思っても、自信がないしプレッシャーがあるので何回も調べまくって、検証してレビュー書いて、解説とサンプルコードも書くみたいな感じだったので労力もやばかった。
ただ、色々と棚に上げたりして頑張ったのでPythonとかJavaScriptとかは経験年数以上の詳しさをもった気がする。でも本当は、僕も他人にレビューもらったりとかして楽にノウハウ欲しかった(悲しみ)。
オブシーの時は結構色々教えてもらったので良さがあった。
見かけ上の出力が大幅に下がる
これは特に、導入初期に顕著。
一つのコミットにたいして10個くらいの指摘がついて、その上でコミットの粒度やコミットコメントそのものもやり直しさせられるので、何も知らない人だと見かけ上の出力が下がって進捗が出ていないように感じる。
もちろん、どうにもまずいコミットが入り込むことが減るので、長期的に見た場合の作業進捗は増す。ちゃんと増した。
だけど、そもそも乗り気じゃない人たちにとって、これは反対する理由の一つになるのでうまく宥める必要が出てくる。
レビュイーのモチベーション低下
これは先の出力低下にも関連する。
学校教育とプログラミングは相性が悪い そして適性とかの話 - タオルケット体操でも書いたが、一般的なレビュイーの意識としてはレビューがつくことはテストでバツをもらうのと等しい行為なわけで、そういう考え方をする人だと、レビューされるのがめんどくさい or 怖いみたいな感じになってしまい、作業効率が落ちたりすることがあるかもしれない。
僕はどうしてもこういうのは苦手なのだが、アプレッソの小野さんのいうような「ひよコード」みたいな文化を作っていくことで緩和出来るのかもしれない。
美しくないコードをuncodeとかクソースなどと呼ぶ人もいるようですが、これらの言葉は開発者を傷つけるので、最近アプレッソでは、あまり美しくないコードを「ひよコード」、美しくない箇所は「ここがピヨピヨしてる」と表現するようにしています。未熟だけど伸び代はあることを意味しています。
— 小野 和俊/Kazutoshi Ono (@lalha2) July 5, 2012
また、もらった意見の中で衝撃をうけたものとしては
「レビューをもらうとコードを書き直さないといけないので時間がかかる」
というものがあった。
当然のように、レビューで弾かれるコードというのは、解読が困難なスパゲッティコード、設計がまずいコード、そもそもバグってるコード、パフォーマンスに悪影響のあるコード、テストがないコード……などである(はず)。
そして、プログラムのソースコードというのは常に、新しく書くよりも修正する方が困難である。自分で書いたものですらそうだし、他人のものはなおさらだ。
そこをレビューで事前に弾くことで、将来のコストを安く先払いしているというのが常識だと僕は勝手に思い込んでいたため、ロクに啓蒙活動をせずに導入してしまった、というのが原因であろう。また、たまたま活発に意見をくれるタイプの人達が言わずとも理解していてくれたので、舞い上がってしまい全体に目が向かなかったというのもある。
我々は、エンジニアの理想系セカイと世間の了解には大幅なギャップがあることを常に意識する必要がある。
ちなみに同じような意見は、二つのプロジェクトそれぞれでもらった。なので、次に同じようなことを企てる場合は些細なことだと自分では思っても、導入の意義をちゃんと周囲に周知する必要があるという反省を得た。
あと、あんま関係ないけど、良かれとおもってやってる事にたいしてなんかウザいみたいな雑な感想を投げられるのはコミュ障の僕でもかなりのダメージを受けて心が折れそうになるのでヤバい。
ここに関しては、Twitterでいうイイネ! 的な、褒めまくる行為を全面に押し出して「レビュー = テストの採点」みたいな構図を中和すると良いよ! というようなアドバイスを頂いたので、最近はなるべくそこらへんを意識するようにしている。
わかりやすいイイネ! 機能がついたレビューツールとかがあったら導入してみてもいいかもしれない。
御意見番化
これはコードレビュー実施のアンチパターンの一つでもあるけど、僕が御意見番
みたいなめんどくさい存在になってしまって、「なんか言われたからとりあえず直す」みたいな感じになってしまう人が出てきてしまった。
レビュアー、レビュイーの性格や、技術的な知識によるパワーバランスとかがどうしても影響してしまうのでとても難しい問題だと思う。
相互レビューみたいな文化がそもそも存在しているとこうはならないのだが、僕みたいに孤軍奮闘状態で導入していくと、これは絶対に避けられない問題だと思う。現状は割り切るしかないという結論。
どこまでレビューしていいのかわからない
どこまで細かいレビューを、という話ではなくて、単純に量の問題です。
とにかく全てがヤバいコードをコミットしまくる人がいた場合、どこまでレビューすればいいのかわからなくなることがあるというはなし。
前提として、バグが入ったり、今後の機能追加が著しく困難になるようなコミットは後で僕が困るので止めたいんだけども、重要なのに絞ったとしても一度に10個以上のレビューをつけないといけないような状況の場合、相手も辛いだろうけど僕もクソ辛かった。
多分、そこまでレベルに差が出てる場合は勉強会とかしてある程度以上レベルが上がるまでコミットを待ってもらうのがいいんだろうけど、「面子を潰さないように」とかそういういまいち僕には理解できない気遣いとかが出てくるので本当にしんどい。
レビューの指摘自体を理解してもらえない
これは設計レベルのレビューをする際によく出てくるのですけれども、例えば「クラスって便利関数と変数を集めておけるアレでしょ?」みたいな認識しかない人に対して、「Controllerが太りすぎなのでモデルに持っていこう」「横断的関心事はdecoratorを使って実装していこう」「継承よりも委譲」「モナドは宇宙」みたいなレビューをしても「お、おう」みたいな雰囲気になる。
なので、僕がサンプルコードを書いたりだとか、色々するわけだけども。
もちろん、最初のうちは僕も喜んで教えるわけだけど、当たり前のようにある程度以上に高度な概念を一発で理解出来る人なんていないわけで、似たような説明を何度も繰り返しているうちにだんだんレビュアーが疲れてしまう現象があると思う。
ここの学びも、ヤバい級の人が書くコードレビュー論に欠けているところで、そもそも設計方法で議論が白熱するようなチームはみんな自発的に勉強する系の人なので、例えばMVCうんたらみたいな話も速攻で通じたりする。でもMVCなんて、大抵のプログラマーは「あー、なんか名前はきくよね」みたいな認識なのですよ。なんで、そもそもオブザーバーパターン知らない人は入社試験で落とされますみたいな会社の人はそこらへんおもいもよらないのでは? 感。
今思うと、これはコードレビューで全てを解決しようとしていたのが原因なわけで、勉強会とかを開催していい感じにするやつも併用すべきだったのかなーと思った。
時々、なんの前提知識もないのにちょっと説明しただけで9割以上理解しちゃう人とかいるけど、そういうチート行為はちょっとズルいとおもった。
とにかく疲れる
やることが多すぎなので疲れる。
組織的な評価制度の重要性を説いている人がいたけど、これは妥当だとおもった。
いくら有意義とはいえ、こんだけしんどいことやって何も評価されないんじゃやる気出ないと思う。なので給料増えたりとか、壁からお寿司が出てきたりとか、定期的に美人のお姉さんがよしよししてくれるとか、そういうがないとあんまやりたくない。
揉める
いけないとわかっていても、疲れるとついついレビューが雑だったり、目つきが悪くなったりとかして喧嘩みたいになることがあった。
エンジニアコミュニティ的なマサカリwwwwwwとか言ってるのは僕は個人的にはあんま好きじゃなくて、僕としてはすぐ切れるコミュ障怖すぎでしょみたいな感じしかないんだけども、いざコーディングしてるとだんだんと僕も頭がおかしくなって切れやすくなったりとかするのでプログラマー怖い。
まとめ(今後)
コードレビューは有意義
某SIerみたいにトンチンカンなレビューをしない限りはプロダクトの品質は良くなる
でも死ぬほど疲れたのでしばらくレビュー普及活動はしばらく休みたい
望まれていない環境で導入する場合、憎まれ役になる可能性が高いので頑張れ
謙虚さを失うとチームの人間関係が崩壊することを肝に銘じておく
だけどうまくいくと、逆に仲良くなったり感謝されたりするよ
意義をちゃんと説明しないと「やらされてる感」になる人が出てくる
チームの技術力アップについては、各自が勝手に勉強しまくるエンジニアばかりみたいな恵まれた環境でもない限りは、勉強会などを併用しないとしんどいことになる
「勉強する気は一切ないけどお前らに面子をつぶされたくない」みたいな人がいる場合は絶対揉めるので政治頑張って
ペアプロやってみたい
ヤバい級の人とペアプロしてみたい。どういう風に思考してコード書くのかみてみたい。
室見さん的な人に厳しい指導(意味深)とかそういう体験を希望していきたい。
以上です。
導入にあたって参考にしたブログとか
http://d.hatena.ne.jp/naoya/20140313/1394664578
http://blog.livedoor.jp/lalha/archives/50495777.html