はじめに
この記事は「フィヨルドブートキャンプ Part 2 Advent Calendar 2021」の19日目の記事です。Part 1もあります。
最初書こうとしていたネタがいまいち筆が進まなかったので今日急遽このテーマにしました。
最近のフィヨルドブートキャンプのチーム開発のプラクティスでは現役生同士でレビューをしているので、自分がレビューする際にやっていることや気を付けていることをただただ書きなぐって「レビュワーになってこれからレビューをしないといけないけどレビューって何すればいいの・・・」というフィヨルドブートキャンプ現役生の助けに少しでもなればいいなぁと思っています。
読んでほしい人
- これから初めてレビュワーを担当する現役生
- レビュワーをすることに苦手意識を持っているエンジニア
- レビュワーをするのが大変そうでいつも他の人に任せてしまっているエンジニア
- どんなことを考えながらレビューをしているのか知りたい人
注意事項
- 自分も常に全てできているわけではないです
- 全部が全部万人に合うわけではないです
- 「これいいな、やってみよう」と思ったものを見つけて試したり自分にあうやり方にカスタマイズしていったりしてもらえれば
先に伝えておきたいこと
この後色々書いてますが、あくまで自分の例です。
レビュワーをするときはあまり難しく考えず自分やチームの知識が深められれば OK、ちょっとでも学びを得られたら大成功、くらいの精神で気楽にやってほしいです。
バグを見つけようと気負う必要もないですし、気の利いたコードの書き方を提示しないでも大丈夫です。
「何も指摘できそうにない・・・指摘できる自信がない・・・」という場合はわからないところやちょっとでも気になった箇所について質問してみるだけでも OK です。
それでスキルや知識が向上すれば万々歳です。
やっていること
自分が使っているエディタでコードを読む
レビューをするときは基本的にローカルのマシン上で普段使っているエディタ、自分の場合は RubyMine を使ってコードを読んでいます。
GitHub の Pull Request 上でも読むことは可能ですが、ローカルのエディタで読むことには以下のメリットがあります。
- 変更箇所以外のコードを把握しやすく、エディタ次第ではコードジャンプも利用できる
- Diff が見やすい
- typo チェッカー、Lint のチェック、型チェック(TypeScript 等の場合) 等が利用できる
一方、レビューしたいけど別の作業中でコミットしていないファイルが手元にあることはよくあります。
そういう場合は、作業中のブランチに tmp のコミットを積んだり stash したりして現在の作業状態を退避しておきます。
@onk さんのブログにあるように TODO.txt を書いておくか tmp のコミットのコミットメッセージかに残しておくかをやっておくとレビューが終わった後に作業を再開する(場合によっては翌日だったり翌週だったりになる場合もある)ときにすごく便利です。
参考
概要・背景 → 全体の差分 → コミットごとの差分の順に確認
レビューでコードを読み進めていく手順としては以下の順番で進めています。
- Pull Request の Description や関連 issue を読んで概要や背景を把握
- Pull Request 全体の差分を確認
- コミットごとの差分をコミットメッセージも含めて確認
他の人がどうしているのかはわからないですが、自分の場合は各コミットのコミットメッセージまで確認しています。
ここに実装・修正の意図や理由、経緯、参考にした情報等がしっかり書いてあると、必要なコミュニケーションの量も減るのでレビューするときにすごく助かります。
Pull Request の Description に書かれていれば十分、という主張もありますが個人的にはコミットメッセージにもきちんと情報を残しておいてほしいです。
コミットメッセージがしっかり書かれているとレビューが終わった後もバグ調査の際や後に別の人(将来の自分含む)がその場所のコードを読むときに役に立ちます。
もし修正理由を書けるほどコードを理解していない場合はペアプロ・モブプロをして理解を深めつつコミットメッセージを整えていってもいいかもしれないです。(これはあくまでジャストアイディアです)
コミットメッセージの良い書き方についてはいいブログ記事やスライドがたくさんあるので参考リンク先を読んでみてください。
参考
コードには How
— Takuto Wada (@t_wada) September 5, 2017
テストコードには What
コミットログには Why
コードコメントには Why not
を書こうという話をした
実際に操作してみる
自動テストがある場合でも、一度は実際に手動で動かしてみて確認してみるのをオススメします。
ここで動かす際は、他の色んな機能と組み合わせて動かしてみたりコードを読んでいて気になった条件分岐のところを確認してみたりこんなことしたらどうなるだろうという意地悪な使い方をしてみたりするのがコツです。
アプリケーションにもよりますがまれによく仕様の見落としや思わぬバグを見つけられたりできます。
どんな人間にもミスや見落としは必ずあるので、実際に操作して確認するときだけは「信頼しても信用するな」の精神で自分も他人も信用しない意地の悪い人間になって色んな操作をしてみてください。
気を付けていること
いいところを探し、褒める
レビューを受ける側も人間なので、指摘ばかりだと気分が落ち込んでしまいます。
なのでレビューというプロセスの中で一度は相手のことを褒めたり称賛したりするようにしています。
人はネガティブな情報の方が印象に残りやすいので、それを打ち消せるくらいにやるのがコツです。ちょっとしたことでもとにかく大げさにオーバーリアクションに、絵文字や bot 等も使って盛大にやります。
コミットメッセージとか README の更新とかサボられがちなことをきちんとしてるのを見たらもうその人は天才、英雄、伝説ですよ。
テキストで伝えるのが難しければビデオチャットを利用する
これテキストにするの難しいなーとちょっとでも感じたらビデオチャットで口頭と可能ならコードの修正例も添えてやり取りするようにしています。
あとこのときに同時編集可能なドキュメントツールを使ってメモを取りつつやり取りし、終わった後に Pull Request に何らかの形で反映しておくとログとして残せて便利です。
HRT を欠かさず相手が嫌な思いをしないように気を配る
HRT とは
- Humility(謙虚)
- Respect(尊敬)
- Trust(信頼)
の3つを合わせたものです。
HRT についての詳しい説明はここではしません。詳しくは Team Geak を読んでみてください。
Team Geak に
あらゆる人間関係の衝突は、謙虚・尊敬・信頼の欠如によるものだ。
とあるように、ちょっとした謙虚・尊敬・信頼の欠如から人間関係のトラブルは発生します。
そうならないよう、自分は以下のようなことをしています。
- 絵文字や語尾の伸ばし棒(ー・〜)を意識的に使い表現が柔らかくなるようにする
- 言い切りや断定的な表現、強い表現を使わない
- 相手の実装を尊重し自分のコメントはあくまで一意見ということをしっかりと伝える
- IMO や nits、MUST 等を使ってコメントのニュアンスを伝える
あとこれはここ最近得た学びなんですが、自身の心や時間に余裕がなくなると人間どんなに気を配っていても気づかないうちに性格や言動がキツくなります。
なのでそうなる前に、適度に休息やリフレッシュをするのをオススメします。
過去の失敗
コミュニケーションにはとにかくやりすぎくらいに配慮することをオススメします。
自分は現役生時代にこれを知らずに悪気なく行っていたレビューによって人間関係でトラブルを起こしたことがあります。
「エンジニアなら〜することが当然なので」のような言葉を使ってしまっていた記憶があります。
この出来事は今でも後悔しています。ただ後悔しても当時のこの出来事はやり直せません。
ちなみにその時 @komagata さんから「この本を読んでみては？」と教えてもらった書籍が Team Geak です。絵文字や伸ばし棒を使ったテキストの表現方法もこのとき @komagata さんから教えてもらいました。
その後もレビューとは関係ないですが自分の人間的未熟さから気を配っているつもりでも相手に不快な思いをさせてしまい Twitter でブロックされてしまったことも何度かあります。
一度こじれた人間関係を修復するのは本当に至難の業(一生修復できないかもしれない)ですし、その人の性格によっては本当に落ち込みます。
自分の場合、毎回しばらく仕事も私生活もできないくらい落ち込みますし涙が止まらなくなるし自分が嫌いで嫌いでしょうがなくなります。
自分も相手も誰も幸せにならない悲しい結果しか残りません。
ただ、人間関係がこじれる可能性は完全にはゼロにはできなくても、普段から最大限配慮して行動し、至らないところは改善を続けていくことで可能性を少なくしていくことはできます。
少しでも嫌な思いをする人が減っていくことを願っています。自分も少しでも相手に嫌な思いをさせる回数を減らせるよう日々精進していきます。
もし、改善したほうがよさそうな行動をとってしまっていたときは DM 等で教えてもらえると幸いです。
参考
おわりに
半日くらいでガーッと勢いに任せて書いた記事なので色々と至らなかったり雑だったり無駄に暗い話をしていたりする箇所もあると思うので何かあればご指摘ください。
明日は @machida さんと @yana-gi さんです。
