コードレビューで気をつける言葉や行い
コードレビューにstashを使ってます。
こいつはgitのブランチ間の差分に対してコメントをつけることができるツールです。
ただ、ネットを介したコミュニケーションって何故か気が大きくなってしまったり、感情が見えづらかったりで誤解を生みがちです。
特にコードレビューって間違いを指摘するとかあんまり楽しい会話をするわけでもないので、言葉には気を使わないといけません。
今日は自分が気をつけている言葉や行いを上げてみます。
否定しない
def get_name(name) @user.find(name: name) end
☓：getは軽量なアクセッサとして使うのが常識なのでやめて下さい。
◯：findしてることが分かるメソッド名が良いです
いちいち否定する必要はないです。素直にどうして欲しいか書きましょう。
否定しない2 「けど〜」
def search(name) @user.find(name: name) end
☓：悪くはないと思うんですけど、find_userがわかりやすいと思います。
◯：ユーザを見つけて返すことが分かるメソッド名がいいですね
けどがコメント中に出てきたら要注意です。
相手に気を使って持ち上げてるつもりでも、けどが出てきた時点で相手は否定されたと受け取る可能性があります。
これも否定せず自分がどうしたら良いと思ってるかだけ書けばよいです。
比較しない 「の方が〜」
☓：find_userの方がわかりやすいと思います
◯：ユーザを見つけて返すことが分かるメソッド名がいいですね
いちいち他と比較しないでいいです。
これも見方によっては否定に見えます。
これをやる場合はある程度根拠を明示した上でやりましょう。
余計なことを言わない
def saerch(name)
☓：typoしてます。searchです。最近確認が甘くありませんか？
◯：typoです
余計なことを言わない。
完全に喧嘩売ってますね。
上下関係があったとしても、コードレビューの範疇から逸脱しています。
命令しない
☓：このコードは汚過ぎでリリースさせることはできません。直して下さい。
◯：口頭で行うこと
ツール上でコメントするという一方的なコミュニケーションが発生しがちな環境下では、命令と受け取れる言葉はうかつに使うべきではありません。
一方的に命令して言い訳も聞き入れないような人にはもう見てもらいたくなくなるものです。
そのようなことがどうしても言いたければ口頭で時間をかけて「説明」して下さい。
本を有効に使う
users.each do |u| u.save end
☓：uではわかりづらいのでuserとしましょう。
◯：リーダブルコードの24pにもあるように、省略形を使うときはチームメイトが理解できるものが良いですね。
著名な本に書いてあるというのはそれだけで説得力が有ります。
レビュワーが勉強していることも伝わって一石二鳥です。
趣味やフィーリングでレビューしない
☓：この書き方は良くないです
◯：個人的な感覚で申し訳ないのですけど、この書き方はあまり一般的でないきがしてます。
なんとなくこの書き方は良くない気がする。
良い方法は提示できないけど、このコードは良いと思えない。
本にも載ってないんだけどこの書き方は一般的でない気がする。
個人的にこの記法は趣味ではない。
たぶんこの手のケースはいっぱいあると思います。
その感覚や考えはちゃんと伝えましょう。
なんの根拠もないのに良くないと言われても、どうしたらよいかわからないですよね。
炎上しがちで結論がうまく出ない系なので、できればお互いに対面で話し合ったほうがよいです。
具体的に提示しすぎない
☓：このメソッド名はfind_userがわかりやすいと思います
◯：ユーザを見つけて返すことが分かるメソッド名がいいですね
特にメソッド名やらクラス名などの命名に関してあまり具体的に提示しすぎないほうが良いと思っています。
というのも命名は本人の趣味や感覚などが出る部分です。
具体的に「これ」と言われると前述の命令にように見えたり、押し付けがましく感じることもあります。
またレビュワーとレビュイーで趣味がぜんぜん違うと大きなストレスになります。
コーディング規約が守られていないとか一般的な命名から大きくズレているような場合はそのことを伝えてあげればよいです。。
設計レビューは先にやる
コードレビュー時に設計についてグダグダいうべきではありません。
やられた相手は「今更言うなよ」とか「設計レビュー時に言えよ」とか思うわけです。
よほど酷いとか品質に問題があるのであれば指摘もやむなしですが、その場合も一方的にならず対面でお互いの合意を形成すべきです。
他の人の意見にのっからない
レビュワーが複数人いる場合に起こるのですが
まずはじめにレビュワーAが何か指摘したとします。
その意見に乗っかって「俺もそう思う」「その意見に1票」などと言うべきでないと考えています。
これは結構賛否有ると思うのですが、
- レビューは多数決ではない
- 誰がどう考えているかは重要でなく、付いた意見自体が大事
と考えるからです。
もちろん意見が対立した場合やチームの他の人の意見が必要だとなった場合はある程度多数決になっても仕方ないと思います。
また、先に付けられていたレビューの根拠が薄かったり足りなかったりする場合に補足的なレビューをするのは良いと思います。
意見が割れたら対面で話し合う
レビュワーの指摘がどうしても理解できない、納得出来ない、今回はできないと思うことはあるでしょう。
このようにレビュワーとレビュイーの意見が割れた場合は、ツールでのコメントをすぐにやめてお互いの目を見て話をし、合意を形成しましょう。
コメント合戦になると大体醜く炎上します。
捨て台詞を吐かない
決まったことに対して「でも俺は良くないと思う」とかコメントしてはいけません。
とにかく最後に自分が言い返して終わらないと気がすまない人っていますよね。
やられて気持ちいいわけないです。絶対ダメ。
レビューに限った話でもなんでもないですね、これ。
長文を書かない
一つのコメント内に何回もダメ出しをしてくる人、何度も同じところを多角的にダメ出しする人。
ネチネチグダグダ。やめましょう。
説明は大事ですけどできるだけシンプルに指摘を。
文章が長いとそれだけで説教臭さがでてきます。
どうしても言いたいなら口頭で。
指摘が通らなくても切れない
自分の指摘が通らないとブチ切れだす人。
切れるとまでいかなくても、ものすごい長文レスで返してくる人。
ダメです。
上にも書いたように意見が割れたら話をして下さい。
良いコードを見つけたら良いと言おう
良いコードを見つけて、それをチームで共有するのはよいと思うのです。
駄目だしばっかしてても気が滅入るだけですしね。
大事なことだからもう一度いう。お互い向き合って目を見て話せ
これが一番大事なこと。
シンプルな指摘で解決しない問題があれば、それはもうレビューツールで解決するべきことではない。
番外 レビュイー側
レビュワーを信じる
☓：お前の指摘は信じられないから直さない。どうしても直してほしかったら御意見番の◯◯さんに正しいか確認をもらってこい！
これをやり始めると常にご意見番がいないとレビューできなくなります。
チームとして機能してないですね。これは。
もちろんレビュワーもレビュイーを信じていないとレビューが機能しません。
自信がない部分は先に伝えておく
自信がないところについては先に伝えておくと話がスムーズになります。
人って相談されるの好きですから、先に伝えておくと指摘がマイルドになります（経験則ですが）
おわり
なんか最後の方はグダグダな気もしますが、いろいろ気をつけてます。
本当はお互いに殴りあってから友人になるような屈強なチームになれたらいいのかもしれないですねー。
