僕がコードレビューの時に気をつけていること
いきなりがっつりとした記事を書くよりは慣れるために軽めの話を。
こんにちは、すーです。
今回は普段僕がコードレビュ(GitHubのPull Requestのレビュー)の時に気をつけていること、大切にしていることを書いてみます。
(※以降話は主にGitHubのPullRequest(PRと略します)上での話がメインだと捉えてもらえますと)
自分がレビューをするとき
まずは自分がレビュワーだった場合に、気をつけていることを。
・ PRの概要、関連するIssueの内容をしっかり読む
まずはじめにPRの概要や、関連するIssueの内容を確認します。
何も見ずにいきなりコードに目をやってしまうと的外れなレビューをしてしまう可能性もありますし、コードの善し悪しだけを追いがちになってしまいます。
どういう意図や背景があってこのPRが出されたのかを確認するのはとても大事になります。
・ コメントに`[MUST]``[NITS]` `[IMO]` `[Q]` のような項目を付ける
自分が特に相手にここを見て欲しい、という意見や質問がある場合には、コメントの先頭にこれらの文字を付け加えています。
それぞれ、
[MUST]: 絶対直してほしいなというときに付けます。強い指摘になるので、言葉遣いには少し気をつけます
[MUST] 通信エラーのハンドリングが不足しているので処理の追加をお願いします!
[MUST] 不要な変更がコミットされているので取り消してほしいです!
[IMO]: 自分ならこうするかな〜、自分はこう思うのだけどどうだろう?というときに(In my openionの略です)
[IMO] `何かしらのコード` みたいに書いたほうがスッキリすると思います!
[IMO] この処理はxxxというクラスに移したほうが責務的には良さそうです。
[NITS]: 細かい指摘や、軽い修正をして欲しいときに(nits pick)
[NITS] ここのコメント、不要なら消しちゃっても良さそう。
[NITS] `console.log` 残ったままになってる 👀
[Q]: Questionの略で、ここ質問したい!という時に付けます。実装で不明瞭だった点や、そもそも自分の知らないことについて聞いたりします。
[Q] ここの処理は非同期で実行される可能性はありますか?
[Q] xxxって機能(書き方)があるんですね、具体的にどういうものなのでしょうか?
伝えたいことがより伝わりやすくなるのでおすすめです。
チームのレビューのルールで決まっている場合もあるかと思います。
もし今のチームで全く使われていなくて使い始めるぞ!という場合は、IMOとかの意味がわからない方がいるかもしれないので予め説明しておくと良いかもしれません。
・良いコードには良いとコメントする
自分が素直に良いと思ったところには良いとコメントします。
絵文字とか絵文字とかだけでも良いですし。。
コードレビューってついつい粗探しになりがちで、良くないな部分を見つけてはコメントしてしまいがちなのですが、
自分では思いつかなかったような良いコードや、前回指摘した部分に気をつけつつコードを書いてもらえた時、
自分の悪いコードを直してもらえた時に褒めたり、感謝を伝えるのもすごく大事になります。
(ちょっと黒塗りばっかりになってしまいましたが...、 💯とかついているだけでも受け手は嬉しかったりします)
・自分が一度レビューしたPRはマージorクローズされるまでは見届ける
ちょっとうまい言葉が出て見つからなかったのですが、
自分がApproved以外、コメントやRequest Changedを出した場合は、最後まで付き添ってあげたほうが良いと思います。
「ここの書き方おかしいです、直してください!」
「xxx使わないでください」
と、自分の言いたいことを言うだけ言ってあとは放置…は良くないので、コメント後に自分が指摘した箇所が直っていたら再度レビューをしてApprovedをしましょう。
誰かに指摘してもらったものを修正したら、指摘をしてくれた人に見てもらいたいはずです。
自分がレビューされる時(してもらうとき)
今度は自分が書いたコードをレビューしてもらうときについて。
次のような事に気をつけています。
・PRの概要をしっかり書く
他人のコードを読むって凄い頭をつかうので、「どうしてこのコードを書いたのか」という情報をしっかりPRに書いて伝えることで、
相手の負担を減らすことができます。
・変更前後のログやスクリーンショットを貼る
あなたがもしサービス開発をしているエンジニアで、アプリケーションの新しい画面を作ったり、既存の画面のUIを変更したりした場合は、変更前後のスクリーンショットを添付してあげると良いです。
・参照すべき情報のリンクを貼る
PRの概要には、このPRで解決される(したい)問題に関する情報のリンクを貼ると良いと思います。
・関連するissueのリンク
・障害レポート
・スプレッドシートの情報
・UIを構成するにあたって参照したデザイン(SketchやFigmaのリンク)
といった情報があると、レビュワーの助けになります。
・適切なレビュワーの指定
・今回直した修正箇所に詳しい人がいる場合はその人をレビュワーに指定する。
・修正箇所を念入りに見て欲しい場合は複数人指定して見てもらう
・誰でも良い場合は特に指定せずに見れる人に見てもらう(あるいはランダムに指定するような運用をする)
といった具合で使い分けると良さそうです。
この部分に関してはチームでの運用方針によって変わりそうですね。
「チーム」を意識する
・他の人が出したレビューに気づいたらなるべく早めにレビューする
そういう気持ちを少しでも持って臨んでいると結果チームとしての開発のサイクルも早まるんじゃないかなと思います。
ちなみに僕が携わっているKomercoの開発チームだと、`[WIP]`の状態になっているPRを除いて、3日以上何もリアクションもなく放置されているようなPRは今までになかったです。
それくらいチームとしてPRをできる限り早くレビューするというのが根付いているんじゃないかなと思っています。
(僕だけが思っている、だったらどうしよう・・・😇
・自分のレビューに気づいてもらえないときは口頭やチャットで見てとお願いする
昨今ではSlackにGitHubの通知を流したり、Jasperで通知を受け取ったりすることも可能なので、割とPRの存在には気づいてもらえる環境ではありますが、
それでも作業に集中していて気づかなかった、気づいたものの「あとで見よう」ってなった後にそのまま忘れてしまうこともあります。人間ですから...
そんなときはチームのメンバーに口頭でも、Slackのメンションでも良いので「レビュー見て欲しいです!」くらい軽くでもいいので声を掛けると良いです。
たったそれだけのことですが、それだけでレビューをすぐに見てもらえたりします。
さいごに
レビューをするときもされるときも、意識を少し変えるだけでコードレビュー、果てはチーム開発が円滑に回るようになるので、
少しでも「あ、これいいかも!」「改善してみようかな!」って思えるものが見つかったら是非試してみてください!
「自分もこれ気をつけている!/大事にしている!」「というものがあったら引き続き大事にしていってください🙏
(軽めのはずが3000字くらいになってしまった...)