hayasaki

気持ちよくコードレビューする為にやりたい事

2018/12/21

Table of Contents

状況
レビューのその前に
レビューの仕方について
- ■レビュー依頼側として
- ■レビューアーとして
参考資料
おまけ

状況

Fusicでは基本的にGitHubを利用して開発行っていますが、diffが視覚的に分かるのでレビューがやり易く、コードの質はもとより、コーディングスキルの向上に大いに貢献していると感じています。

しかしながらGitHubを利用したレビューは文章のみになりがちです。
もちろん口頭で伝えることもありますが、書くということができますので、残すという意味でも文章で書くことが多いです。

そして、一般的にレビュー依頼者はスキルが低く、レビューアーはスキルが高いことが多いです。

結果どういうことが起きるかというと、ぶっきらぼうだったり高圧的な文章を出してしまい、ちょっと気マズイ雰囲気が起きることがあります。

本来の目的の事が出来なければそれはレビューにはならないですし、
あの人にレビューは出したくないな。。。みたいなことにはならない様にお互いが気持ちよくレビューする為にやりたい事というところで、私が社内で実践されているのを見て良いなと感じているものを書きたいと思います。

レビューのその前に

WIPの話になりますが、そこにプラスして仕様を貼り付けるということをします。

WIPについては WIP新しい開発手法を読んでいただければと思います。

具体的なタスクを書き出していくのですが、ここには仕様に関する部分が無いなと感じています。

ということでその部分を書こうと思うのですが、よく見るところではIssue番号や、長い長い文章だったりします。
私の経験上ですが、Issueはバグ報告を登録している場合が多いかなと思っています。
設計書なんて無いです。または設計途中で内容をIssueに登録してるよ。なんてケースは今回は除きます。

話を戻しますが、
開発時点ではお客様と決めた仕様書があると思います。
が、それを別にIssueに起こす作業をするのは個人的にはイケてないなと思っています。

理由は

情報が仕様書とIssueとの複数箇所での管理が発生する。
記載内容が双方で異なる（ミス）場合が発生する可能性がある。
そしてそこに取られる時間コストです。

手っ取り早く、仕様書のキャプチャを貼ったり、仕様書のエントリポイント（ファイルパスやURL）を記載したほうが早いと思っています。

こんな感じです。
仕様書に無い情報があり、とはいえ書いた方が良い。と判断されるものは書きますし、可能なら仕様書を更新したいところです。

ちょっとレビューとは違う話ですが、これってどういう話のやつだっけ？という会話のやり取りを減らすTipsかと思っております。

レビューの仕方について

長くなりましたが本題です。

■レビュー依頼側として

1.細かくレビューを出しましょう

開発において、○○を作る。というものが多いと思います。
そこではDBのテーブルから作る？画面から？JSも？MVC全部を実装？？？
多すぎてレビュー依頼するときchangefilesが結構出てくることが想定できます。（レビューする側はどこを見ればよいか分からない状況が起きます。）
また、その実装思想が最初からミスっていたら、以降のコードも無駄になる可能性があります。

実際にはPRのタスクをできるだけ分けて開発するのがいいでしょう。

画面部分
JS処理
バリデーション部分
ビジネスロジック部分
テストコード部分

等々と、粒度を細かくしてPRを出すのがレビューする側も安心してできますし、修正範囲で少なくリカバリし易いでしょう。
しかしながら、タスクを分けるのが難しいと感じる場合もあるとは思います。
その時は、潔く一部分だけ（上記だと画面部分だけとか）の途中の状態でレビューしてもらいます。
レビューアーはその部分だけを見れば良いわけですからは絶対に楽です。
依頼側もミスや心理的不安を早い段階で取り除けます。

2.PRのタスクを書いた時点でも見せる

コードを書き始める前にそもそもコレ作るけど、問題無いですかね？です。
経験が浅い人や途中からプロジェクトに入った人はなるべく行った方が良いと思います。

3.レビューを出す直前に気を付けたい事

とりあえずレビューお願いします！というレビューはレビューアーからすればどこをどう見てほしいのかが分かりませんので控えましょう。
レビューしてほしい部分があれば、changefilesの行レベルで、こういう理由考えで書いてみたのですが良いか悪いか意見が欲しいです。くらいで書いた方がいいです。
何故そのように実装したかの理由があればそこも含めて書きましょう。

レビューする側は全てを把握しているわけではありませんし他の作業をしているかもしれません。頭のスイッチに時間が掛かります。
明確に自分がどこを気にしているかが分かるとレビューする側も精度が良くなります。

■レビューアーとして

1.まずは心を仏にしましょう

レビューはどうしても悪いところに目が行きます。そこを指摘してしまいますが、受ける側の気持ちを考えましょう。
お客さんにメールするときくらいの感じで、指摘の文章にぶしつけ感がないか気を付けましょう。

（悪い例）
ここは良くない、なんでこんな風に書いたの？

（良い例）
ここは、ちょっと○○な理由からこういう風にしたほうが良いですねー 😄
※実際のコード例があるとなお良し

相手も人です。いっぱい飛んでくると心が折れます。

2.そのコードが良いと思って書いている

基本的にレビュー出す側はそれが一番良いと思って書いているので、それ以外の実装方法が思いつかない（知らない）というケースが殆どではないかなと思っています。
文字としては柔らかく、決して人は非難せず、ぶっきらぼうに書かず、指摘する理由を柔らかく述べて、場合によっては解法を添えてあげるくらいの返しをしたいと思っています。
また、指摘だけではなく良いところも見つけましょう。この実装いいね！とかどんな些細なことでも言われた方は嬉しいものです。