はじめに

この半年くらいで初めて本格的にチーム開発を行い、今では日常的に GitHub の Pull Request を使っています。
チームの方々には、基本的なことから応用的な部分まで様々な観点からレビューをしてもらって、大いに勉強になりました。

ただ、時には「新人にとっては厳しいレビュー」をいただき、1 人で傷つきモチベーションを落とすこともありました。
もちろんそれは悪意のあるものではなくて、新人とレビュワーのスキルのギャップによって意図せず生み出されてしまうものです。

そのような不幸なレビューによって苦しむ新人が減ることを願って、新人を不用意に傷つけてしまう恐れのあるレビューをまとめていきたいと思います。
新人教育の場に少しでも役に立てていただけると嬉しいです。

前提条件

今回の対象とする「新人」は、本格的な開発経験が1年未満の方を想定しています。

個人で少しプログラミングはしてきたけれど、チーム開発は未経験の方がイメージです。

※ レビューの受け取り方には個人差があります。
※ 今回のレビューの例は全てフィクションです。
※ 所属組織に一切関係はありません。
※ 悪用は絶対にしないでください。

傷が深いレビュー

以下のようなレビューをしてしまうと、新人プログラマに大きなダメージが入る可能性は高いです...。

公式ドキュメントだけを渡す

個人的にですが、一番辛いレビューがこれです。致命傷です。たとえば以下のような内容です。

公式ドキュメントに「コントローラ」について詳しく載っているので、それを参考に修正してみてください。
https://readouble.com/laravel/5.7/ja/controllers.html

レビューの内容は正しいし、レビュワーにとっては当たり前ですぐに理解できることなのだと思いますが、新人にとってはかなり辛いです。

あくまでも公式ドキュメントは基本的な使い方が記載されている場所であって、それを応用して書くということがかなり難しかったりします。

また、フレームワークの全体像がつかめていないのに、ドキュメントの1点を読んでも何がなんだかわかりません。

しかし、真面目な新人の場合は、「自分がこれを読み込んで頑張れば良いだけ！」と抱え込んでしまい、それで上手くいけば良いのですが、多くの場合は何も解決できなくて途方もない時間を溶かしてしまいます。

可能な限り公式ドキュメントだけを提示するレビューは避けてください。

改善例

公式ドキュメントに「コントローラ」について詳しく載っているので、あとで一緒に読みながら修正していきましょう。
https://readouble.com/laravel/5.7/ja/controllers.html

公式ドキュメントを読む習慣を作ることは非常に良いので、最初のうちはペアプログラミング（ペアリーディング）をしながら、「公式ドキュメントの読み方」を育てていけると良いと思います。

無意識に煽ってしまっている

気づかずに新人を落ち込ませてしまうことがあります。事故の場合もあるので注意です。たとえば以下のような内容です。

こういう書き方があるのは知りませんでした。どこかに参考になるものはありますか？
<a link="example.co.jp" target="_blank">

上記の例は明らかに <a href="example.co.jp" target="_blank"> が正しいのですが、レビュワーがミスを気づかせるためにこういう書き方をしたのだと思います。

しかし、新人としてはなんとなく煽られている感じがして気持ちが良いものではないので、素直にミスを指摘してくれたほうが良いのではないでしょうか。

こういうレビューが続くと、新人が失敗を極度に恐れるようになることもあるので、「新人に自分で気づかせる」という意識もほどほどにするのが良いかと思います。

改善例

aタグはlink属性を持たないので、 <a href="example.co.jp" target="_blank"> が正しいです。
aタグの他の属性についてはここにまとまっているので、書くときは参考にしてみてください。
https://developer.mozilla.org/ja/docs/Web/HTML/Element/a

ミスを的確に指摘してもらいつつ、再発を防止するための方法を教えてもらえると、新人の励みになります。

「新人にミスを気づかせる」よりも、「新人に同じミスをさせないようにする」というレビューのほうが良いと思っています。

ちゃんと確認した？って確認する

新人はたとえ確認していたとしても、「すみません確認不足でした」としか言えないので辛いですね...。

この実装については、ちゃんと確認しましたか？もう少し考える余地がありそうです。
const xyz = Object.assign(xy, {z: 3})

新人としても、Pull Request を出すからには可能な限りの確認をしているはずです。
ノーヒントで「確認した？」とだけ聞くと、調べる時間だけが経過して、なかなか答えにたどり着けないこともあります。

同様に、直接聞きに行った時に「ちゃんとググった？」って聞かれるのもキツイですね。
そもそも何を確認すれば良いかわからない、何をググれば良いのかわからない状態なときもあるので。

ペアプロなどを通じて、レビューには出てこないような思考の欠陥を知ることも大切だと思います。

改善例

Object.assignは破壊的関数なので、この後に元々のxyの値を使うことができなくなります。
参考：https://qiita.com/yujilow1220/items/978b811a9910a9516bb4
ES2018で追加されたスプレッド演算子で結合すると良いと思います。
const xyz = {...xy, z: 3}

実装は動いているけれど、コードの品質に問題がある場合は特に確認が難しいです。
まずは、新人の今の実力を認めて、その成長を促進させるようなレビューができると良いでしょう。

かすり傷のレビュー

そこまで大きく傷つくことはないと思いますが、累積すると新人のモチベーションが削られてしまう恐れのあるレビューもまとめておきます。

プルリクエストの範囲を広げる

プルリクエストをテンポよくマージしていけると嬉しいものですが、レビューで修正内容を広げられるとけっこう困ったりします。

商品一覧ページのこの部分と共通化できると思うので、一緒に修正してもらえますか？

一気に修正したい気持ちもわかりますが、品質への影響がない場合には、issueなどにまとめておいて、別のプルリクエストで修正しても良いと思います。

1つのプルリクエストに対して何日もレビュー＆修正を繰り返していると、精神的に辛くなってきませんか？

プルリクエストを小さくし、その分マージを多くして成功体験を積み重ねられると、新人のモチベーションも上がっていくでしょう。

改善例

商品一覧ページのこの部分と共通化できると思うのでissueにまとめておきました。これがマージされたらそちらもお願いできますか？

プルリクエストの回転数を上げて達成感を得られるような状況も作っていきましょう。

場合によっては、プラスアルファの部分はメンターのほうで担当して、コードの書き方のお手本を見せたり、その部分のレビューを新人にお願いするなどして、多角的に作業を進めるのも良さそうです。

わからない単語を並べる

プログラマ独特の言い回しが理解できない時もあります。なるべく平易な言葉を使うことをおすすめします。

コードの品質を担保するために、マジックナンバーは定数に切ってください。また、このファイルは抽象度が高いので、もう一段階層を掘ってください。

新人にとっては、何を言っているのかわかりそうでわからないです。

こういうレビューは、なんとなくわかった気になって進めることもできますが、修正して出したら間違っていた、ということもよくあります。

シャレオツでナウいレビューも良いのですが、わかりやすい表現を使ったほうが結果的にお互いが助かります。

改善例

変更があった場合に対応しやすいように、数字を直接入力するマジックナンバーは避けて、定数を設定して使うようにしてください。また、このファイルは特定の用途で使われるので、"profile"ディレクトリのようなものを作って、そのなかに置いてください。

まあ、ちょっとレビューは長くなりますが、レビューと修正が何往復もするよりも健全でしょう。

マジックナンバーなどの言葉は、初回は意味の説明と同時に記載すると伝わりやすいです。
事前に会話可能であれば、一度言葉の説明をしてから以後使うようにしても良いかもしれません。

レビュー以前の問題

こんなにレビューに気を遣う必要があるのか？と思った方も多いのではないでしょうか。

もちろん、レビュー以前にエンジニアどうしのコミュニケーションが取れていて、新人の心理的安全性が保たれていればなんの問題もありません。
レビューで多少は苦しんだとしても、直接コミュニケーションを取ることで精神的に追い込まれることは無いでしょう。

ただ、新人との挨拶もそこそこに、文字でのコミュニケーションが主体となる職場もあると思います。リモートワーク主体の職場ではよくあることでしょう。

そうなった場合、新人は職場の人々の雰囲気もつかめず、慣れない環境の中で、新しい技術に向き合っていかなければならないので、なかなか苦しい戦いが待っています。

「新人には大変な思いをして学んでもらって、そこから成長してほしい。」
そう思う方もいるかもしれませんが、そんなことをして新人に辞められてしまっては一大事です。そして結局は、ベテランエンジニアが人手不足で苦しむことになってしまいます。

新人をレビューでなるべく傷つけないでください。新人教育における不幸が少しでも減ることを願っています。

追記

予想以上に多くの方に記事を読んでいただけて、大変嬉しく思います。念のために追記をさせてください。

私自身は、現在弊社で丁寧な指導のもと、充実した日々を送っています。
十分な心理的安全性が保たれていて、わからないレビューに対しても気軽に口頭で聞きに行ける環境を作っていただいています。

もし、こういった環境が無いままに上記のようなレビューを受けたら大変だろうな、という想定のもとで記事を書きました。
ですので弊社に誤解のないよう、よろしくお願いいたします。