コードレビューのはなし
エンジニアをやっているとコードレビューをしたり受けたりすることが日常的ですが、レビューは人間関係に絡むのでお悩みの職場も多いのではないでしょうか。
そんな中、ぼやっと思ったことをここに書いておきます。
人間関係が浅いうちはレビューコメントにタグつけるといい
レビューコメントにタグつける運用あるじゃないですか。
[MUST] ループの中で毎度このオブジェクトを初期化すると初期化コストがかさむのでループの外で一度だけ初期化するようにしてください。
[WANT] 要素の数が事前に判っているので、詰め替え先のスライスはmakeに長さを指定した方が省エネなので良いと思います。
[ASK] 好みの問題なんですけど、初期化した後にフィールドに代入するんじゃなくて、初期化のときに一緒に渡せば良いのではないかと思いました。
こういうの、いちいちつけるの面倒くさいと思うんですけど、レビューア・レビューイ*1の人間関係が浅いうちは付けておいた方がいいと思いました。
なぜタグをつけた方がいいかというと、MUSTだったら対応しないとapproveもらえないだろうから対応する、でもWANT/ASKだったら忙しいから・自分の好みじゃないから無視するといったやる・やらないの判断がしやすくなるからです。
そもそも、指摘事項にぜんぶ対応しないとapproveがもらえないレビューは厳しいです。
レビューアに権力があって、レビューイが服従するみたいな関係だとレビューを受けたくなくなります。
レビューアのコードの書き方の好みまで合わせなければならないというのはヤバイと思います。
だからレビューアはまず自分の指摘が絶対直して欲しいのか気が向いたら直して欲しいのか区別して指摘を書く必要があって、それを示すのにタグが役立ちます。
タグつけてレビューやってるのに直す・直さないで意見の一致が図れなくてレビューでもめたり、ストレス溜まるという人は、レビューやっても意味ないでしょうから、メンバーと喧嘩するなり、自分がチームを出て行くなり、相手を追放するなり好きにやってください。
多分その人たちは反りが合わないんでしょう。管理職に相談した方がいいかもしれません。
人間関係が浅いうちを卒業するのはいつなのかというと難しいんですけど、同じチームで一緒に仕事し始めて3ヶ月以内とかだったら互いに気ぃ遣い合った方がいいんじゃないでしょうか。
Githubのプルリクのresolve機能は使った方がいい
指摘を出してレビューイがプログラム直したら再レビューしますけど、レビューアは指摘したことに対する修正内容に満足したらresolveボタンを押して、その話が決着したことを明らかにしてあげると良いです。
同様に、レビュー受ける側も「ここは解決済みだ」と思ったらresolve押してしまって構わないと思います。
レビューアもレビューイもお互い適当にresolve機能使ったらいいと思います。せっかく用意されてるので。
「勝手にresolveするなよ」と互いにもめるようなら、話し合うなり何なりしてトラブらないよう努力してください。
チーム開発なんて互いに気持ちよく働いていく努力しないと成立しません。
間違いの訂正なのかリファクタなのか判るコメントを書いた方がいい
レビューアはプルリクを見ていて間違った修正をしていたら訂正を求めるコメントを出します。
そういう間違いの訂正がコードレビューの主な仕事になっていくと思います。
レビュー受けてる方も基本は指摘をもらったら自分の修正内容ないし方法が間違っているんだなと思います。
でも、ついでにリファクタしてほしいときもありますよね*2。
レビューイにリファクタさせたいときは「これは間違いではなくて、リファクタを依頼している」と判るようコメントを書くとお互い捗ります。
さっき書いた「レビューコメントにタグをつける」とかはその辺にも役立ちます。
さいごに まず良い職場を確保した方がいい
コードの書き方でもめる職場はツラいので、まず気持ちよく働ける同僚が入ってくれる良い職場を作りましょう。
まともにレビューが成立しない人がなぜか同じチームに入ってくるという場合、まずその状況に問題意識を持ちましょう。
逆に、自分が知らない職場に入って行く場合、一緒に働く人とうまくやっていけるかよく考えて職場を選びましょう。
たぶん、自分よりスキルが上の人たちがいる職場に行くのはあまり問題にならないでしょうが、誤って自分より程度の低い人がいる職場に行ってしまうと本当に後悔します。
程度の悪い人とは働かないのが一番だとわたしは思います。
ちゃんとしたチームがあってこそ良いレビューとは何かという問題意識が生まれます。
問題の優先順位を履き違えないよう気をつけないといけません。
最後までお読みくださりありがとうございました。
