タイトル詐欺です大変申し訳ありません。
No.1レビュワーは語弊ありです。PRのdiscussionで指摘を入れている数がチーム内で一番多い、というだけですすみません。
そもそもレビュワーにランキングなんてつけるのはナンセンスです（白目）
はじめに
筆者は2021年10月に株式会社ニジボックスに入社し翌月11月に現PJ（プロジェクト）へアサインされました。
入社時・アサイン時点では実務経験数ヶ月足らずでレビュー経験も0、そこから1年弱でチームNo.1（自称&驕り&過言）レビュワーになるまにでにやったことや意識したことをまとめていきます。
対象読者
- 新人のエンジニア
- コードレビューをしたことがない人
- コードレビューに苦手意識やためらいを持っている人（レビュワー目線）
書くこと・書かないこと
- 書くこと
- 自分語り
- ポエムよりのポエム
- 書かないこと
- 技術的なこと
前提
私は歴史のある大規模サイトのエンハンス対応PJにFE（フロントエンドエンジニア）職として参画し、主にフロント面のコーディングを担当しているチームに所属しています。
ABテスト・改修・リニューアルなど複数の案件がウォーターフォール形式で同時に走り各FEがそれぞれコーディング部分を担当し、また自分の案件対応とあわせて他FEが担当している案件のレビューも行う、という業務内容となっております。
このようなフローの中、No.1レビュワー（自称&驕り&過言）になるまでに意識したこと、実践したことなどをまとめていきます✍️
3つのステップ
自分のレビュワーとしての成長を振り返ってみると、3つの段階があったように思います。
- STEP1（最重要）: とにかく指摘をする
- STEP2: 実装内容を把握した上でコメントする
- STEP3: 上から目線でレビューする ← イマココ
これらの段階をまとめていきます。
（以下PRのdiscussionのことを「指摘」と表現します。必ずしも指摘をしているわけではなく質問形式のコメントなども「指摘」に含みます）
STEP1（最重要）: とにかく指摘をする
アサイン当時は「コードレビューってなに…」「なに書いてあるかわかんない…」「なに指摘すればいいかわかんない…」「指摘するの怖い…」「わかんないけどとりあえずLGTMしとこ…」みたいな状態でした。
アサインから1ヶ月ほど経ってもこのような状態が変わらず「このままではいけない。メンタルから変える必要がある。」と思い、まず最初にやったことは「とにかくなんでもいいから指摘しまくる」ということでした。
これが自分にとって最も重要な取り組みだったと感じています。
【目的】
- 息をするように指摘できるようになる
- 「間違った指摘をしてしまったらどうしよう」をなくす
- チーム内に「どんどん指摘をするメンバー」という認識をつけてもらう
【やったこと】
- その月に出される全レビュー依頼のうち９割以上をレビューし、それぞれに必ず１つ以上指摘を入れる
アサイン3ヶ月目あたりから自らの成長施策として上記を4ヶ月くらい続けることにしました✍️
[実際のPR確認依頼の例]
こんな感じで専用スレッドにPRの確認依頼が飛んできます。
月20~30くらいのペースでPRが作られますが、そのうち9割以上をレビューしていきました。
【意識したこと】
- とにかくなんでもいいから指摘をする（最重要）
- 「自信ないな」「この実装どうなってるんだろう」と思ったら質問形式で指摘する
- まずは「指摘すること」の練習だと考える
とにかくなんでもいいから指摘をする
ここでは「指摘をする」ということに重きを置き、とにかくなんでもいいから指摘することを意識しました。
「よりよい実装方法の提案」や「不具合につながる実装の検知」などのレビューをできるレベルには全然いなかったので「タイポやインデントのズレ、その他リファクタレベル」「デザイン実装に問題がないか（コードレビュー力なくてもできるため）」などをメインで指摘していました。
目的は、息をするように指摘できるようになることです。
自信がない場合は質問形式で指摘する
「間違った指摘をしてしまったらイヤだから自信がないときは指摘するのやめよう」と思いがちなのは私だけではないと思いますが、そこは質問ベースで指摘するようにすると抵抗なく行えたので意識して実践していました。
まずは「指摘すること」の練習だと考える
「指摘すること」の練習だと考え、一旦指摘の質は考えずいわゆるnitsな指摘でもガンガン行いました。
「『そんな小さな指摘するなよ』て思われたらどうしよう」と思う方もいるかもしれません。
大丈夫です。「自分がレビュワーとして成長できた方が結果としてチームのためになる」というメンタルでいきましょう。
ただし...
「明日納品」というPRに細かい指摘を「練習だぁ〜！」と言っていくつもいくつも投げるのはあまりにGuiltyすぎるので...☠️
STEP2: 実装内容を把握した上でコメントする
さて、STEP1を2ヶ月くらい実践していると指摘することへの抵抗感は少なくなり指摘の数も増えてきていました。
STEP1の目的は達成できつつあったのですが、いつまでもデザイン指摘やリファクタレベルの指摘をしていては肝心のレビュー力が伸びないとも感じていました。なんてったってあんまりコード追えてないんですから...
ということで次のステップに進むべく「しっかりとコードの流れを追い実装内容に対して指摘する」ということを行うようにしました。
「当たり前では？」と思われるかもしれません。そうなんです、その当たり前ができるようになることが目的でした。
【目的】
- 「リファクタレベルの指摘」から「実装内容への指摘」へステップアップする
- 息をするように「実装内容への指摘」ができるようになる
【やったこと】
- コードの処理を追った上で実装内容に対して指摘する
STEP1と比べると抽象的ですが、要はしっかりとコードを読み下す、ということをしました。
/**
* @param {Element | null} some element
*/
const echo = (elm) => {
if (!elm) return;
console.log(elm);
};
(() => {
const elm = document.querySelector('.elm');
if (elm === null) return;
echo(elm);
})();
例として、上記のようなコードがあったとします。DOMから要素を取得しそれをコンソールに叫ぶという謎コードです。
STEP1の状態の私ではリファクタレベルでの指摘しかできなかったため、「要素の存在確認が=== nullと論理否定演算子によるfalsyチェックで統一されていないのが気になりました><」のような指摘をしたかもしれません。
しかし処理をしっかり追ってみるとelmは即時関数内で存在確認が行われた上でecho関数に渡されているので、そもそもecho関数内での存在確認は不要です。そのため、STEP2の私は「echo関数内でのelmの存在確認は不要です」と指摘することができます！
例ではコードがあまりに短いので簡単ですが、ソースのボリュームが多くなったとてしっかりと処理を追って指摘することを心がけました。
【意識したこと】
- コードを下から読むようにする
- 議論ベースの指摘をする
- 手元でソースを動かしてレビューする
コードを下から読むようにする
STEP1では「コードを読む」となったら馬鹿正直に上からつらつら読んでいたのですが、チームの先輩に「私はコードは下から読んでいる」と教えていただき、ありがたく自分も真似するようにしました🙏
これも人によっては当たり前かもしれませんが、コードは基本『定義→実行』という順序で書かれているので、特に命令型言語のJavaScriptの場合下から読んだ方が流れを追いやすい、ということに気付かせてもらい意識するようにしました。
議論ベースの指摘をする
機能要件に対する実装方法に"正解"はなく、あるのは"意図"だと思っています。
なので「私だったらこう考えてこのような実装をしますが、どう思いますか？」や「このような実装方法もあると思いますが、どっちがいいでしょうかね？」のようにレビュイーに対して議論する形の指摘を行いました。
手元でコードを動かしてレビューする
この頃のコードの見方ですが、GitHubの「File changed」画面にてソースの差分をブラウザ上で見るという方法でレビューしていました。ただこの方法では差分のみの確認となり如何せん読みづらく、またなによりコードを自分で動かせないので深く追えない、という自分には不向きな方法でした。
ということでしっかりソースブランチにチェックアウトし、自分の手元のエディタでコードを動かしながらレビューするようにしました。するとかなり深くコードを追えるようになるため「まあぱっと見だいだいOKでしょう🙆♂️」みたいな雑LGTMがかなり減ったと思います。
またVSCodeの場合、GitHubのPRを確認する際に非常に便利なextensionがあり、こちらもフル活用いたしました。
STEP3: 上から目線でレビューする
STEP2を続けていると取り組み開始から半年ほどで「息をするように実装内容に対して指摘できる」という状態になり、かなり指摘の量も質も向上しました。
ただ、チーム内には私よりもっともっとレビューが上手な方がいらっしゃり、特に「要件漏れや不具合につながる実装への指摘」（このような指摘を『クリティカルな指摘』と呼称しています）という超重要指摘をよくしてくださっています🙏
このようなクリティカルな指摘はSTEP2の「実装内容を追う」だけではなかなかできず、さらにもう一歩ステップアップする必要があると感じました。
ということでSTEP3として「上から目線でレビューする」という取り組みを行っています。
語弊たっぷりの言い方ですが、どういうことかと言うと「コードを表面レベルで見るのではなく、少し高度をあげて上空から見渡す」みたいなイメージです。そう言う意味での上から目線、です🙄
コードを表面的に見るのではなく
- 少し上から、要件を踏まえて見る
- 少し上から、なにを実現したいコードなのか踏まえて見る
- 少し上から、不具合につながるパターンはないか見る
などのようにレビューを行い、クリティカルな指摘ができるようになることを目指しています。
これは現状できていないので、がんばって実践中という段階です💪
【目的】
- クリティカルな指摘ができるようになる
【やっていること】
let hasClass = false;
const elms = document.querySelectorAll('.elm');
elms.forEach((elm) => {
if (elm.classList.contains('some-class')) {
hasClass = true;
return;
}
});
if (hasClass) {
console.log('Either element has the class!');
}
例として上記のようなコードがあったとします。DOMから複数の要素を取得し内１つでもsome-classというクラスを持っていたら、コンソールに向かって「Either element has the class!」と叫ぶ謎コードです。
リファクタする点もなければ、処理を追った上でも特に問題や指摘する点はなさそうです。STEP2ではこのコードになんの指摘もしないでしょう。
しかし少し上空からこのコードを眺め「なにを実現したいのだろうか」を考えると、「配列（風オブジェクト）から特定の要素があるかどうかを調査している」ということになります。
その場合「配列の中の少なくとも1つの要素が合格するかどうか」を判定するためのメソッドArray#someが存在し、こちらを用いれば配列内を全探索する必要がなくなる場合があり、よりよい実装と言えます。
ということでSTEP3では
「const hasClass = [...elms].some((elm) => elm.classList.contains('some-class'))の方が適しています」
という指摘をすることができます！
（「クリティカルな指摘」の具体例ではないですが...）
【意識していること】
- 案件の要件を踏まえてレビューする
- 「そのコードによって実現したいことはなにか」を見る
案件の要件を踏まえてレビューする
案件の要件をしっかりと把握していないとコードの表面的な指摘しかできないため、結局クリティカルな指摘をするには要件の把握が必要なんだと思い知ってきました。
要件の把握はコードを見るだけでなくWFや要件定義を読む必要があり、必然的に1つのPRを見る時間が長くなってしまいますが、これはもう仕方がない...クリティカルな指摘をするためには...
「そのコードによって実現したいことはなにか」を見る
要件を踏まえた上で、ではその要件を実現するためにどのような実装をしているのだろうか、という上からの目線でレビューします。
コードの内容を追っているだけでは気付きづらい「xxxの考慮がないから要件が漏れている」や「そもそもxxxでよいのでは？」といった指摘ができるようになり、たいです（願望）
また複雑なコードは処理を追うことに必死になってしまいがちですが、俯瞰した目線で「そもそもなにを実現したいのか？」を見るとコードも読みやすくなるし指摘もしやすくなると感じています。
STEP0: レビューによるダメージ軽減策の提案
本筋ではないのでおまけ程度にこの辺に乗っけておきます。STEPの順序が狂っているのはお許しください。
入社して１~２ヶ月くらいの私、まだSTEP1すら実践できてない段階くらいの頃、ふと気付いたことがあります。
それは「メンバーがレビューによって結構ダメージを受けている」ということです。精神的に。
もちろん、チーム内にハラスメントや厳しい口調でのレビューをするメンバーはいなかったのですが、それでもやはり「自分のコードに指摘を受ける」ということに少なからずダメージを受け、特に1つのPRで指摘の数が多くなるとメンタルにきている感が目に見えてわかるようになっていました。（「メンタルが弱い」と思われる方、みんながみんな強いわけではないのです🏋️）
ということで超絶新入り&レビュー数0件マンの私、生意気にも提案を持ちかけました。それが
「指摘コメントの語尾に『><』つけましょう」
というものです。
目的は指摘によるレビュイーのダメージ軽減、それとレビュワーの「なるべく柔らかい口調で指摘しようとコメントを考える時間」の削減です。まずはこちらをご覧ください。
- 「こちらコードが冗長になってます。修正お願いします。」
- 「こちらコードが冗長になってます。修正お願いします><」
このように、語尾に『><』をつけるだけでかなり印象が柔らかくなりますよね。そうですよね!?><
また「『><』つけるだけ」というルール（？）なので、レビュワー側も無理に柔らかい言葉を探す必要がなくなります。
目に見えて効果が出るような施策ではないので、そのほどは、と聞かれるとなんともですが...
きっと多分少なからずダメ軽にはなったかと思います！（願望）
ちょっと本筋から外れたお話でした。
閑話休題。
まとめ
以上私のNo.1レビュワー（自称&驕り&過言）までの成長日記を振り返ってきました✍️
対象読者の項目でも記載したように、この記事はレビューに苦手意識を持っている新人エンジニアの方へむけて書き上げました。
最もお伝えしたかったことは「STEP1」の目的である「息をするように指摘できるようになる」ことです。
これはある程度数をこなすだけでできるようになることだと思います。
レビューに苦手意識を持っている方は必ずしも技術的なスキルがないというわけではないですし、そういったスキルのある方がレビューをしないというのは非常にもったいないことだと思います。
また、技術的にまだ自信がないからレビューが苦手という方も、「他人のコードを読む」というのはそれだけで難しく経験が必要なことですので、その経験を積むためにもまずは「息をするように指摘できるようになる」まで多少無理やりにでも取り組んでみてはいかがでしょうか！
おわりに
最後までお読みいただきありがとうございます。
こういった新人の頃の悩みやそれに対する取り組みはつよくなればなるほど忘れて行ってしまうものかと思い、備忘もかねて記事にまとめました。
誤字や脱字の指摘、またレビューに関する考え方の違いなどコメントいただけるとありがたいです🙇♂️
おまけ
なにをもってNo.1と豪語しているのかというと、単純に指摘している数が一番多い、ということです。ただそれだけです。質もひっくるめたら全然No.1じゃないと思います🙄
（流石にこれだけ指摘しているのでそれなりに質も上がってきているとは思いますが...）
私がアサインされた11月から1年弱の間のチームメンバーそれぞれの指摘数を集計し以下にまとめました。
なお結構雑に集めてきたので正確とは言い切れないです。が、おおよその数はあっているでしょう。
赤線が私です。12月の途中あたりからSTEP1を取り組み出したという感じです。
ちなみに毎月たくさん指摘しているAさんはクリティカルな指摘も連発してくださるので、間違いなくこの方がNo.1です。
記事を書くにあたり集計してみたら月によっては私が一番指摘をしていたので、せっかくなので（？）詐欺タイトルにしました。タイトルには強い言葉を使え、と誰かに教えてもらった気がするので👹
この記事をわたしのチームの人が読んでいないことを祈ります。