コードレビュー

コードレビュー，慣れるとできるけど，いきなりdiffを渡されて，どうぞ見てくださいと言われてもよくわからないと思う．
やりましょうというのはいいけど，ただむやみに読んでもうまくいかない．変更がある程度大きくなるとdiffだけ見てもよくわからないので，いろいろ見ることになる．
僕はいつも以下のようなことを無意識にやってて，うまくいってる気がしてる．GitHubのPull Requestの仕組みを使ってる前提で．

1. Discussionをさらっと眺めてどういう問題を解決したいのか見る
2. Commit Statusを見て，テスト通ってることを確認する
3. Commitsタブで1コミットずつブラウザの新しいタブに開く
4. 全部クリックし終わったら古い順に1コミットずつ読む
5. 気になる点があったらエディタとかにメモしておく．あとで書き直されるかもしれないので，まだコメントしない
6. 全コミット見終わったらFiles Changedで変更をまとめて見る
7. diffに現れないあたりのコードが気になったら，View file押して完成したファイルを読む
8. すでに似たコードとか使えるはずの便利関数がある気がしたら手元でgit grepとかする
9. 挙動が気になったら実際に使ってみる．クリックして使ったり，Perlだったらテスト追加してみたりJSだったらステップ実行してみたりする
10. ここが良いとか，変とか，質問とか，感想とか，かわいいとか，Diffに対してコメントを書く．感想くらいでも書くとそのコメントをもとに議論できる
11. 必要なら適宜ミサワを貼るChrome拡張ã‚„tiqavを簡単に貼るChrome拡張を利用しておもしろ画像を貼りまくる
12. Discussionに戻って，この設計は良いとか，理想的にはこういう形になるとうれしいとか，これに似てるとか，diffに現れない設計についてコメント書く
13. マージしてよいと思ったら「よさそう :shipit: 」とか書く．GitHubで:shipit:すると齧歯類の絵が出る．キネシスのマクロで1キーで :shipit: 出るようにしてる
14. :shipit:だけだと迫力が出ないのでLGTM.inからおもしろ画像を探してきて貼る
15. 相談とか，疑問とか，まだマージしてはいけないと思ったら，ここはどうなんですかとかコメント書く
16. forでpushしてたらmapとgrepでもいいですねとか，こんなのみんなよく使ってるとか，僕だったらこう書くとか，こう書くとかっこいいとかコメントする．コメントするだけで，その通りに直してもらうことはしない
17. pull-req-labelっていうChrome拡張を使ってるのでレビュー完了状態にする
18. IRCとかで「見た (URL)」とか書いて見ましたよってお知らせする
19. IRCでコードについて話したり席まで行って話したりする

毎回こんなちまちまことやってるわけじゃなくて，1行CSS変えただけとかだったら2秒くらいdiff見て良さそうってコメントして終わる．
コミットを順番に見ていくと書いた人と同じ気持ちになれるので良い．いきなりdiff見ると，ここはこういうことですか？って気になるけど，コミットメッセージにだいたい書いてある．がんばって書いたけどうまくいかなくてrevertする様子なんかを見て楽しめる．

そもそも設計がまずいようなPull Requestが来ることはあまりなくて，先にどこを変えればよさそうとか，こういう仕組みを作ろうとか，この機能はどのクラスの責任かとか，よくコード書く前に相談してる．エンジニアとデザイナーは全員京都にいて，席も近いのですぐ会話できる．
1つのPull Requestで完璧にする必要はなくて，正しく動くならマージしていこうという雰囲気が出てる．コーディング規約守っていて，正しく動いていれば良い．コーディング規約も普通に書いてれば守れるようなのしか入れてない．微妙なところは続けてissueを作ってリファクタリングしたり，次に手をつけるときに書き直したりする．スケジュールとの兼ね合いもあるので，コードの見た目は悪いけど，時間はないのでまずはこれくらいで，みたいな話をすることもある．完璧に仕上げてる間にmasterが進んでマージできなくなると寂しいので，動くならまずマージしてしまって，そこから落ち着いてリファクタリングのissueを作ることが多い．とはいっても，読みにくいコードとかを書く人はそもそもいないので，そんなに変なコードをどんどんマージしていくということはしていない．いまは複雑な実装だけど，こういう抽象を導入すればより良くなるはずなので，やりましょう，みたいな．納品して完成みたいなものではなくて，毎日リリースしてるくらいで，ちょっとずつ変わっていくものなので，最近は良くなってきたとか，だんだん悪くなってきたとか，そういう捉えかたをするべきだと思う．庭とかいきなり完成させる人いなくて，ちょっとずつ手入れしていくと思う．

コードレビューを通してお互い勉強して高めあっていこうという雰囲気があって，たぶん本を紹介するのは良くて，コメント欄でがんばって説明するよりは，この本のこの章によるとこう書いてあるので読んでみてはって言うほうが早い．
せっかくなので，コードレビューの際によく紹介する本を紹介しておきます．

リーダブルコード ―より良いコードを書くためのシンプルで実践的なテクニック (Theory in practice)

薄い本で，読んだらすぐ使える．関数の大半がifに入ってたら先にreturn ifとかして抜けるほうが読みやすいですねとか，そういう技が書いてあって役立つ．コメントにはどういうことを書くべきかも述べられていて，意図や事情を説明するとか，欠陥があるところには，FIXMEとか書いて，ここは問題があるって書いておくと後で役立つとか．Code Completeにも似たこと書いてあるけど，こっちのほうが手っ取り早い．

オブジェクト指向入門 第2版 原則・コンセプト (IT Architect’Archive クラシックモダン・コンピューティング)

オブジェクト指向入門 第2版 方法論・実践 (IT Architects' Archiveクラシックモダン・コンピューティング)

これは一番好きな本で，めちゃ分厚いのだけど，下巻の最初のほうに，まともなコーディング規約とは，とかそういうことも書いてあって，その章読むだけでも役立つ．「必要のない限りは使うべきではない」という規約はおかしくて，使おうとしてる人は必要あるからそのクラスを使っているのだから，もっと良い書き方がある，とか書かれてていい話．契約による設計の章も良くて，このクラスにはどのような責任があるか，継承と契約の関係とは，といった価値観を学べる．これを読まずにクラス作ったり継承とかしてるのはやばい．

もっといろいろあるかと思ったけど振り返ると上の3冊でなんとかなってる気がする．知識が少ないから同じ本の話をずっとしてる．