レビューを受ける前に僕が気をつけていること2017
昨日、スターウォーズの前夜祭でエピソード8見てきました！ えーすけです！！！
ラストジェダイ最高だったんでみんなエピソード１から全部見て行きましょう！！！！ ネタバレすると、スクリーンで動くポーグがひたすらにかわいい。
この記事は feedforce Advent Calendar 2017 - Adventar の 15日目の記事です！
昨日は、スケちゃんの
GraphQL API をスキーマファーストで開発したいけどスキーマと実装で乖離を起こしたくない - Feedforce Developer Blog
でした！ GraphQL 楽しそう！！！
今回の記事は、コードレビューについてです(サムネ画がポーグなのは全く関係ありませんw)。 最近行った勉強会でレビューについて考える機会があったので、普段の業務の中で日々受けて自分なりにこうしてるよというのをいい感じにまとめれたらなと思っています😀
ちなみにその勉強会はこちら
前提
今の開発チームはバックエンド２人・インフラ１人・フロントエンド２人・デザイナー１人で開発しています。 自分はバックエンドチームとして、先輩と２人で頑張ってます。
今回の話は、基本的にバックエンドの先輩とのやり取りで得たことがほとんどです。
事前準備
そもそも自分がタスクに取り掛かる上で、
- 自分が何を作るべきかを理解しておく
- 0 コメを充実させる
この２点は必ず意識しています。
何を作るべきがを理解する
ここで言う理解には、仕様の理解と機能がどんな使われ方をされるか？の理解とがあります。
仕様については基本的には、ミーティングの中でやりとりをしているんですが
ここらへんは再度自分がやる前に漏れがないかを確認しておくと、スムーズに開発が出来ると思います。
また、機能の使われ方については、実際にユーザーにどう使われるものかのイメージを持っておくことで、気付けるテストケースもあるので非常に重要です。 どう使われるか知らずに作るより、自分が作るものは自信をもって説明できるようにしておきたいですよね。
0 コメの充実
Github の PR を作る時にまず書くのが description いわゆる 0コメ部分だと思います。 自分は、まずは空コミットで push して、やることリスト的な感じで書いていきます。
{対応する issue 番号(あれば)}
## レビュー開始条件
- [ ] 関連 PR がマージされて rebase 済み
- [ ] CI 通ってる
...
## やったこと
- [ ] Schema の更新
- [ ] User モデルの追加
...
## やらないこと
## 実装方針(新規機能とかの時)
まずはレビュー開始条件。 これは主に自分用の TODO みたいな形で使っています。
次にやったこと/やってないこと。 ボリューム感が多くなりそうだったら、やってないことに書いて、 PR として分ける必要がありそうだなぁとかはここで妄想しています。また、やったことがふわっとしか書けなかったら、自分のタスクへの理解が足りないので、この時点で一旦相談しています。
最後に実装方針。0コメ書き始める前に確認しておいたポイント、ミーティングで決まった仕様や、今回のざっくりの実装方針をまとめていきます。細かい実装のことというより、ざっくりと処理の流れを書いて頭の整理をしています。
セルフレビュー
0コメ書いて実際に実装が一段落付いて、レビュー条件が全部クリアできたら、レビューに出す前にセルフレビューをしています。 今年度１番頑張ってやっているのがこのセルフレビューです。
セルフレビューで何をするかですが、一通り眺めるのではなく、他の人が書いたコードの気持ちで実際にレビューしていましょう。 セルフレビューでは３つのことを意識しながらやっていきます。
- コメントで自分の気持ちを先出し
- テストケースは網羅できてる？
- 自分の認識を明文化していく
一つずつ説明していきます。
コメントで自分の気持ちを先出し
ここでこんな気持ちで実装したんや！というのも合わせて、コメントに書いていきます。
特にクラスや変数の命名はかなりレビューコメントを受けるポイントだと思うので、自分の場合
- こんな意味を込めて、xxxx にしました
- 似たような機能の gem が xxxx なので、xxxx にしました
- 他の候補として oooo も考えましたが、xxxx にしました
今感じのコメントを付けることが多いです。 なんでこれをしておくかというと、「こっちの名前の方がいいんじゃない？」ってコメントに対して、コメントするより、自分がこうしたいからこうみたいな気持ちを先に伝えておいたほうが言いやすさがあると思っているからです😌
もちろん、パイセンの提案の方がよいなぁと思ったら、そっちにしますし、それ以上の場合は口頭で話すようにしています。
ちなみに、こんな感じのコメントは先輩にも書いてもらうように依頼しています。 これは完璧に勉強目的です🙏ありがたや〜😭
テストケースは大丈夫？
次に見るポイントは、テストケースが網羅出来ているかです。
- Context の切り方大丈夫？
- 境界値でちゃんとテストできてる？
- 以上/以下/より大きい/未満/nil/型違い ...etc
- 正常系、異常系の書き漏れがない？
- 重複しているテストはない？
- Model と Controller で同じことをテストしてない？
- スタブが変な感じになってない？
- ちゃんとバグがあった時にも気づけそう？
などなど。せっかく自動テストを書いているならば、テストケースも抜けがないように確認していきたいですね。
あとは、関数として共通化できる部分がないか？ N+1 とかなってない？ とか普段レビューで見るポイントはしっかり見ていきましょう 👀
自分の認識を明文化していく
※ここは筆者がやりたいけど、苦手なので結局どこまで理解できたの？って１番指摘を受けている部分です。
セルフレビューの時にやっておくといいのが、認識の明文化だと思います。 ここで一番やりたいのは、自分がやったことを整理して伝えることです。
新しい gem を導入したり、ベンチマークを取って検証してみた際に、
- どんな使い方をするもの？
- テストデータはどんなものを使った？
- 結果はどうだった？
これらのことをまとめていきます。ポイントはそのコメントを見た人がコメントを追っていけば自分の環境でも再現できることが大事です。
更に発展させるとしたら、自分の認識が複数の状態を混在させてないか？という点も大事です。 特にバグ修正関連の PR を作る時はここ大事だと思います。
- どんな事象が発生したからバグが起こっているのか？
- そのバグの再現性は？(テストデータ込み)
- ライブラリの問題なら、どの部分が悪そう？
こんなコメントをバシッとまとまって書いてあると修正 PR をレビューする側から見ても分かりやすいですね。 また、複数の問題が絡まっていそうな場合、一つ一つを独立した問題として検証が出来ると、相手にも分かりやすく伝わる気がします。
この問題の切り分けが難しいので、毎回先輩にはどんな感じで調査進めたかは聞くようにしていて、真似していきたいポイントです😇
[番外編]レビューする時に気をつけていること
レビュー受けるばかりではなく自分がすることも当然ながらあります。 むしろ、自分が１つ PR 作っている間に ２〜３個 PR が作られていることも稀によくあります。
自分がレビューする時に気をつけているのは
- 自分だったらこう書くのになんでこう書くのだろう？
- コードを読んで、自分の理解をまとめから聞く
この２点です。
自分だったらこう書くだろうな
自分が普段業務で使っている言語は Ruby/Python が多いので、人によって上手く書けるかが大きく変わってくる言語だと思っています(個人的に) 設計の仕方だったり、メソッドの使い方だったりで、自分の思った実装と違った場合、なんでこう書いたかを最近意識しています。
この時、実際にコードを書く必要は全くないと思います。 自分が知らなかったメソッドを使ってたら、 irb とか pry で実際に挙動を試してみることもやっています。
コードを読んで、自分の理解をまとめから聞く
これは、認識の明文化と同じ話にはなりますが、ただ単に「わからないんで教えて欲しい」だけだと、先輩も困るので、一旦自分の中で整理して「ここまでは分かったんですけど、ここが分からんので教えて欲しい」的な感じ話し始めると、先輩に質問するのも楽になりました。
自分は話しを整理するのが苦手なので、ひたすら付箋に自分の頭を書き出して整理するように心がけています。 やっぱ頭の中だけで考えるのと、視覚的にも整理が出来るのは大きく違いそうです。
ちなみに、先輩に話し始めて自分で話しているうちに自己解決する場面が結構あります。 人に話していく中で自分の理解が整理されて解決することは多々あるので、言葉の力って凄いなと日々思っています。
人に話して解決するのをテディベア効果とも言うらしく、テディベア持ってないんでスヌーピーに話しかけるしか😏
まとめ
今回は、自分がレビューを受ける(する)時に気をつけていることをまとめてみました。
実際問題、自分がこれ全部毎回出来ているかは微妙なのですが、毎回意識してやっていくことで、レビューに対する苦手意識もなくなってきました。
レビューは相手がいてこその部分もあるので、お互いにどんな感じで指摘して欲しいとか言い合える関係や場所があるかも大事かと思います。 (実際、私もこんな感じで書いていただけるとレビューしやすいのでよろしくお願いします的なのを話しました)
レビューはたくさん受けると辛いこともありますが、よりよいプロダクトにしていくためには絶対に必要なことなので、来年は自分からもガンガン提案出来るように勉強頑張るぞ💪💪💪
明日は、今の私が所属しているプロダクトである dfplus.io の PM ガッキーさんが io リリースからの１年を振り返ってという記事の予定です！！！楽しみ！！！
