コードレビューするのが怖いと思っていたエンジニアが半年間コードレビューを経験して思った 10 のこと
これは pepabo Advent Calendar 2016 - Qiita の14日目の記事です。 昨日は id:Fendo181 さんの 日報サービス「DuPo」を作った話でした！
それは、今からちょうど半年前のこと。
海の香りと共に暑い夏がやってくる ... 甘酸っぱい青春が再び来るのではないかと予感させる ... そんな季節でした。
開発チーム内で行っていたスプリントレトロスペクティブの時間に、チームメンバーから「そろそろコードレビューをやってみよう！」と提案があり、それから本格的にコードレビューをやり始めることになりました。
早いもので、あれから半年が過ぎました。
今宵は年の瀬ということもあり、ふりかえりを目的として半年間コードレビューを積み重ねたことで僕の中で起きた考えの変化や感じたことについて 10 個書き出してみることにしました。
教育関連に興味がある方や組織の成長を考えている方、僕のようにコードレビューに対し臆病になっている方に向けて、何か感じていただければ良いなと思って書きましたのでどうか最後までお付き合いいただければ幸いです🙏
目次
- コードレビューするのが怖かった。できないと思い込んでしまっていた。
- どんな内容でコードリーディングを行ったのか
- コードレビューに自信がないときはどうすれば良いか
- レビュー依頼時にレビュアーのことを考えるようになった
- レビューは取り込むもの
- チーム間で意識の摺り合わせを行うことでレビューが円滑に進むようになる
- 研修時の絶対良感が根底に染み付いている
- インデント修正は人間がやるべきではない
- コードレビューの目的
- 楽しく円滑にコードレビューする簡単な方法
- 最後に
1. コードレビューするのが怖かった。できないと思い込んでしまっていた。
お恥ずかしい話、当時僕はコードリーディングすらまともにできず、個人の開発業務ですら上手くいかないお粗末な状態でした。そのため、そんな状態の僕がコードレビューなんてできるはずがないと思い込んでいました。なぜなら、他人のコードレビューをするためにはまず個人の開発業務の中で培うコードリーディング力が必要だと考え、コードリーディングが出来て初めてコードレビューができると自分の中で勝手に学習の段階付けを設定していたためです。例えば、FF的に言うと サンダガ を覚えるにはまず サンダラ を覚えてからという順番があることと同様に考えていました。
しかし今だからこそ思えるのですが、それは間違っていました。
コードレビューとコードリーディングは縦に学習の段階付けがされているものではなく、それらは並行していて相互的に学習が可能でした。
そして、サンダガ を覚えるには サンダラ を習熟するのではなく、まずは サンダガ を体験してみることこそが学習の近道だと分かりました。
2. どんな内容でコードリーディングを行ったのか
とはいえ、やはりコードリーディングすらまともにできないエンジニアがレビューを行い LGTM としたプルリクエストが本番反映されるというのはちょっと怖いですよね。例えば、家電製品を量産している工場の新人チェック員が「何をチェックするのか全然分かってないけど、とりあえず大丈夫そうだから OK です！俺まだペーペーなんで、なんでも経験かなと思ってます！」といった具合ではちょっと不安です。というか問題です。
そこで僕の場合は、僕の不足分をチームメンバーに補っていただく形でどうにかレビューを体験することが可能となりました。
それでは具体的にどんな内容を行なったのでしょうか？ それはシンプルに次の 2 点でした。
- 僕がコードレビューの一次受けを行う。
- ただし、簡単なレビュー依頼であれば僕だけのジャッジでよしとする。
- 先輩エンジニアがコードレビューの二次受けを行う。
この体制は以下のメリットがありました。
- レビュー不足によってサービスのコードに対し障害を招くような爆弾や技術的負債となりうるコードを反映させることなく安全性を担保できる。
- 僕が判断した善悪のジャッジと技術力の高い先輩エンジニアの善悪のジャッジを直接比較することができる。
- 良いことについては、そのまま継続していくべきだと自信が持てた。
- 悪いことについては、何が悪かったのか、どこが考慮漏れとなっていたのかを知ることができた。
3. コードレビューに自信がないときはどうすれば良いか
なんとかコードレビューデビューを果たした僕でしたが、何回かレビューをしていくとある問題にぶち当たりました。 それは自分のレビューに自信が持てないことです。 なんとか先輩エンジニアのようにレビューをしたいと思っても「本当に僕が言っていることは正しいのだろうか？」と考え込むことがしばしばありました。 そこでチームメンバーにふりかえり会で相談したところ、以下のようなフィードバックをいただきました。
- もちろん違和感を持った理由を明示して「こうするとより良いのではないかなと思いました」と自分で考えて改善案までコメントできたら立派だが、最初はそんなことまではできない。
- 違和感を持つべき箇所は必ずあるので、その箇所について「なぜこのように書いたのですか？」と質問するだけでも良い。
- 最悪スタイルガイドを知らなくても、間違え探しのように他のコードと比べながらコードを見てみると「妙だな..」と気づくことができる。
- 何か疑問を持ったということは、やりたいことがそのコードから読み取れないからであって、必ずどこかに議論や改善の余地がある。
- 違和感を持って提起しただけのコメントだけでも、レビュイーに再考させることで新たな発見が生まれたりすることもある。また、Github 上でのやりとりは Slack 上に通知されるため、これを見たチームメンバーからまた新たなコメントを貰えることだってある。そのような一歩は無駄にはならないので勇気を持って発言していこう。
当時とても納得しましたし「これならやっていけそう！」とテンションが上がったのを覚えてます。 今でもレビューするときには、このとき教えていただいたことを意識するようにしてます。
4. レビュー依頼時にレビュアーのことを考えるようになった
レビューをする立場になったことで、レビューし易いプルリクエストとし難いプルリクエストを区別できるようになりました。 例えば、レビューがし易いプルリクエストの特徴としては以下が挙げられます。(良いPRについては他にいくらでも既出のブログがあるのでざっくり述べます)
- 意図がはっきり明示されていること
- 一つのプルリクエストでやりたいことが一つとなっていること
- コミットの粒度が適切で流れのあるものとなっていること
- 動作検証結果やテスト結果が明示されていること
- 関連する issue や PR のリンクが貼られていること
上記については分かっていたつもりではありましたが、先輩エンジニアのコードレビューをするようになってからは今までより一層第三者目線で一歩俯瞰した考えのもとの設計を心掛けるようになりました。
ただし、上の項目を満たしているプルリクエストは確かに見やすくレビュアーにとっては優しいですが、見やすさや分かりやすさを追求しすぎることで時間がかかりすぎてしまっては本末転倒です。そこで大切なのがレビュイーとレビュアー双方にとって最も効率の良いバランスを保つことだと思うのですが、先輩エンジニア達は皆そのバランスが上手く、彼らが書いたコードをレビューをすることでその微妙な平衡感覚を養うことができたと思います。
5. レビューは取り込むもの
これは新卒エンジニア研修を受けていたときに印象深かったエピソードです。 当時新卒エンジニア同士でコードレビューをするといった研修内容があったのですが、そこで指摘してもらった箇所について「修正しました」とコメントをしていました。 しかし、それでは直せと言われたから「はいはい直しましたよ。これでいいすか？」と言っているような感じになってしまい、レビュアーからレビュイーに対しての一方的なコードレビューとなってしまいます。 そこで研修担当さんに、本来コードレビュー文化のあるべき正しき姿はチーム間で議論をしてサービスにとって最終的に良いと判断したことを取り込むことであり、もしも修正するという内容なのであればそれはレビュアーの意見を取り込むことなのじゃぞと教えていただきました。 それからは 修正 という言葉は使わずに レビューを取り込む という表現を使うようにしています。
6. チーム間で意識の摺り合わせを行うことでレビューが円滑に進むようになる
ある日、チームで行っているスプリントレトロスペクティブにて「JavaScript のレビューについてオーバーヘッドが大きくなってしまっていないか？」という内容が議題に上がりました。 この問題の原因は、JS について統一されたルール(スタイルガイド)がチーム間において確立されていないことでした。
そこでチームのアクションとして JS に強いメンバーが先導する形で朝会後に Javascript-style-guide を５分間だけ読み進めることになりました。
すると、以下のような良いことが起きました。
- 各個人のコードに対する流派のような「こう書くべき」という認識が多少異なっていることが分かり、その場で話し合うことでチーム間のスタイルガイドを確立することができた。
- 今まで背景がブラックボックスとなっていたコードについても、
一般的に最新の理想の環境では「こう書くべき」だがサービス固有の問題によって「こう書くべき」となっているといった作成経緯を把握することで、既存のコードについても理解が深まった。 - チーム間で共通したガイドラインが生まれたことによりレビュアーのコメントが楽になった。
- 例: 「ほら！これ前に一緒に勉強したところだよ〜！」「根拠はスタイルガイドにあります。」などなど。
- 単純に JS 苦手な人の学習にもなりチーム内のボトムアップ効果があった。
- お恥ずかしい話、僕はこの時間で初めて ES5 と ES6 があることを知り、担当するサービスが IE に対応しているため JS が ES5 で書かれているという事実を知ることができました。
チーム間でレビューのオーバーヘッドが大きいと感じた際には、このような手法を試してみると良いかもしれません。
7. 研修時の絶対良感が根底に染み付いている
エンジニア研修時は Ruby on Rails の思想を基調として MVC や TDD といった一般的に正しいと言われる Web アプリケーションの仕組みや開発手法を学びました。 サービス配属後はしっかりとしたフレームワークこそありませんでしたが、長年利益を出し続け会社の収益基盤を作ってきた敬意を払うべき歴戦の勇者のコードを触れるようになりました。
正直最初は環境が変わりすぎて慣れるまでは善悪の判断がつけられず大変でした。しかし、コードが少しづつ読めるようになってくると、研修で学んだことは環境に依存せずに正しいことだと理解できるようになり、まるでジャングルの中でコンパスを見つけたように少しづつ動けるようになりました。(行ったことないですけど..)
僕はこのとき、初めて開発に携わったコードが研修で学んだ正しいコードで良かったと思いました。 なぜなら、悪い環境が犯罪者を育ててしまうことと同じで、レガシーな環境が当たり前となり善悪の判断がつけられない人になってしまうことを恐れたからです。 レガシーは慣れるものではなく改善していくものです。
新人には正しいコードレビューをさせるためにも、善悪の判断がつけられるように絶対良感を養うような新卒研修を行うべきだなと思ったことを覚えています。
※ 絶対良感という言葉はこちらのテックブログから参照させていただきました。 tech.pepabo.com
8. インデント修正は人間がやるべきではない
自明ではありますが、この半年間で僕も強く思ったので自戒の念を込めて敢えて書きます。
人間がやるべきことと機械がやるべきことは区別するべきです。 機械ができることについては人間がやるよりも機械に任せた方が数百倍効率が良く正確なので積極的にやらせるべきです。
レビューについて当てはめると、具体的にはテストの実行は CI に、インデントの修正や指摘については言語毎にあるリントツールを使いましょう。
人間は機械にはできない発想や考えることに時間を費やすようにするべきです。
9. コードレビューの目的
よくコードレビューの話題では 気をつける言葉 や アンチパターン といった内容が挙がります。
しかしコードレビュー初心者にとって大事なのは、まずコードレビューがそもそも何のために行われているのかといった目的を理解することだと思います。
僕の中の一つの答えは 問題vs私達 という構図を常に忘れず、最良のコードをチームで考え、導き出し、残すことでした。 (もちろん属人化を防ぐことや安全性の担保、チームの成長という側面もあると思いますが今回は割愛)
チームの力を最大限に生かし、最大のバリューを発揮させるためにコードレビューは行われています。
そしてその目的を実現させるための手段やテクニックの一つとして 気をつける言葉 や アンチパターン があると思います。
例えばダイエットを成功させるために最も必要なものは、効率の良い最高なダイエット機材や食料といった手段ではなく、まずダイエッターとしての屈強な心を持ちダイエット戦士になってしまうことです。ダイエット戦士の心を持つことで、自ずとその手段としてダイエット機材や食料に手を出すといった順序が望ましいのではないかと思います。
話を元に戻すと、チームで最良のコードを残すという目的を忘れなければ、どんなレビューコメントも「最良のコードを残すために遠慮せずに意見を述べてくれている」と考えることできます。また、レビュアーとしてもそのような心掛けをするべきでしょう。レビュイーとレビュアーの双方が互いに目的を一致させ遠慮せずに歩み寄ることで最高のコードレビューができるのだと思います。
10. 楽しく円滑にコードレビューするための簡単な方法
とはいえ、全ての人が禅問答を解いたような仏の心を持っている訳ではないと思います。頭で理解していても心が納得できていないなんてことはよくある話です。
でも安心してください。それらの問題を解決する簡単な方法があります。
それは 絵文字, 顔文字, 感嘆符を適材適所に使う ことです。
我々は感情を持つ人間なので、例えば合理的故に少し厳しめなコメントとなってしまっていても、末尾に笑顔の顔文字や感嘆符があるだけで安心を得ることができます。 本来であれば、最良のコードを残すために遠慮せずに意見を述べてくれているという相手の考えをトレースして納得するべきですが、人間は論理と感情のパラドックスを持ち合わせているため、目的の他にこのような手段が有効となるんじゃないかなと思ってます。
ただし、全く関係のない絵文字の使用はオススメしません。 なぜなら、相手に意味不明な絵文字を使った理由を考えさせてしまうためです。勘違いや無駄を生むような行為はコミュニケーションコストを高めるだけなので避けるべきです。 僕は入社当初、意味もなく可愛いからという理由で 🗿 を連発して使ったり、LGTM の gif 画像を連投してしまったり、本当に意味のないタイミングで 🏩 の絵文字を使ってしまって相手を困らせてしまった経験があるのですが、今考えると黒歴史ですね。その頃はコードレビューの本来の目的を考えもせずに、ただ面白くてノリが良い方がみんな楽しくてハッピーになると思っていました。
コードレビューは自分一人で完結するものではなくチーム開発の一貫です。 今後もコードレビューの本来の目的を見失わずに、関係者各位の時間と労力を奪わず、かつ適宜いい感じにチームの士気を上げられるようなクリエイティビティに富んだ絵文字や顔文字の使い方を心掛けていきたいです。
とまぁなんやかんや述べましたが、僕が言いたいのは研修時代に受けたこちらの資料が最高ということです 😊 自分がコードレビューをする立場になってから改めて読んでみると、当時とはまた感じ方が違っていて面白いなぁと思いました。
最後に
冒頭でも少し述べましたが、いざコードレビューをやってみるとそれに付随してコードレビュー以外の学習スピードが格段に上がりました。
具体的には、コードレビューに必要とされるコードリーディング力や将来を見据えた設計力、テスト力など、僕が冒頭で述べたいわゆる サンダラ にあたる力のことです。この分かりにくい例えをもっと続けると サンダガ を会得しようと サンダガ の修行を必死に頑張ってたら サンダラ や プリザラ, ファイラ のレベルも気がついたら上がっていたといった体験でした。
圧倒的成長の理由は、先輩エンジニア達による自分よりも綺麗で洗練されたお手本となるようなコードをレビュアーとしての立場で何度も何度も見たことにあるのではないかと思います。先輩方！本当にありがとうございました！
最後になりましたが、かつての僕と同じようにコードレビューの一歩を踏み出せずに尻込みしている方、またはチームの中にコードレビューをしていない未成熟のエンジニアさんがいる方に向けて一言。
綺麗なコードを一からコードリーディングするのも勉強になると思いますが、コードレビューを通してチームのエンジニアと共にプロダクトを造り上げながら実戦で得られる知見は代えがたいものがあるのではないかと思います。
このブログを読んで少しでも何か感じていただけたのであれば、是非この機会にコードレビューを始める、または提案してみるといったアクションをとってみるのはいかがでしょうか 😆 ✨
