はじめに
LITALICO Engineers Advent Calendar 2023 の6日目の記事です。
昨日は、橋本さやかさんさんの、エンジニア組織のHRBPが行った「社内ラジオ」リニューアルPJTの具体的なプロセスと学びでした！
こんばんわ!
LITALICO 新卒一年目の折田です！学校教育事業部でエンジニアをやらせていただいております。
現在、投稿日の前日19時。テスト前夜の如き焦燥感のなかでこの記事を執筆しております
(社内のアドベントカレンダーのルールだと、投稿日の前日が締め切り)
アドカレ初参戦なのですが、今回は、自分がPRレビューをするにあたって振り返ったことを書きたいと思います。
表題にもある通り、チーム１厳しくレビューしていた（はず）ので、今回は、新人が厳しいレビューをするにあたっての心構えや意味について書いてみました！（普段は大口叩くタイプじゃないので、いま震えながら「チーム１」って打ってます）
想定読者
- PRのレビューをするにあたってハードルを感じている人
- そもそも何から学べばいいかわからない新人エンジニア
- 新人育成したい人（にも役立つかも?）
概要
この記事の主題は、ずばり、新人こそPRをチーム1厳しくレビューしよう、ということです！(PRレビューの観点について、というよりは新人がやるにあたっての心構えがメインテーマです)。
これを読んだ方（主に新人エンジニアの方）には、一緒にチーム１厳しくレビューする人になってほしいので、いまからここでみなさんを「説得」します笑
まずは、「説得」するにあたり、issueからはじめよ(issue=実現可能で、インパクトがあること からやるべきということを語っていると解釈しています)を参考に、以下2点について述べたいと思います。
- それが可能なのか = 新人が先輩より厳しくレビューなんてできるの？ 先輩にとって迷惑じゃないの？
- それが価値があることなのか = PRレビューによって新人が何を得るのか?
実現可能で価値があることなんて、みんなやるに決まってますね。
次に、厳しいレビューにおける
3. 具体的なプロセス
について書きます。(レビュー観点の詳細というよりは意識面)
最後に、僕の細かくてしつこいPRレビューに付き合ってくださった先輩方について述べ、
4. どういう環境だと、「新人が先輩のコードを厳しくレビューする」をやりやすいのか
という環境面について、新人視点で述べたいと思います。
前提
- 僕の所属する学校教育事業部のエンジニアチームでは、1つのPRに対して、最低２名のレビュワーが立候補制でつき、レビューしています。そのため、手を挙げれば新人でもPRレビュワーになりやすい環境です。
- この点において体制が異なるチームに所属されている方には、今回の話は部分的にはあてはなまらないかもしれません。ただ、本質は、目上の方が書いたコードを臆せずレビューすることで学びを得よう、ということなので、そもそもPRレビューに限った話ではないです。ご自身の環境に合わせて読み替えてくださると幸いです。
0. 定義
厳しいレビューとは、以下を満たすものとします
- レビュワーにとって不明点がない。完全に理解、納得した状態でのみLGTMを出す。
1. それが可能なのか
この点に関しては、逆に「新人が先輩のコードをレビューする」ことの障壁が何かから考えたいと思います。
それは大きく以下２点ではないでしょうか？
1-1. 知識面や経験面で劣っている新人が思いつくことなんて、他のもっと強い先輩がとっくにケアしている。それより厳しい指摘なんてできないよ、、、。(新人が厳しいレビューをする「能力」の障壁)
1-2. 先輩のコード読むのに時間かかるから、新人がレビューすると開発サイクル遅れちゃう。それに、どうでもいいこと指摘したら迷惑だし生意気かも、、、。(新人がレビューすることが迷惑でないかという「安全性」の障壁)
これらについて、どう対処したのかを自分の経験から語りたいと思います！
1-1. 新人が厳しいレビューをする「能力」の障壁について
まず僕は、PRレビューをすることの役割を考え直しました。
PRレビューは、コード品質を保つことも目的ですが、新人がやるにあたっては、それを通して先輩のコードから「学ぶ」ことも大きな目的です。
結論から言うと、「学ぶ」という観点においては、新人は、能力がないからこそ一番厳しいレビュワーになれます！
つまり、レビューには高度な指摘をする能力が必要という前提をまずは取っ払いましょう。
基本的に新人が一番不明点が多いはずなので、必然的に「なぜこういう実装になったんだろう」という疑問がチームで一番多くあるはずです。僕は、こういった疑問点について、もちろん自分でも調べますが、それでもわからない場合はPRレビューの場で積極的に質問しました。
そして、不明点を残した状態で「LGTM」なんて言わないようにしていました。だって、よくわからないものに対して「Looks Good To Me」は嘘つきですよね笑
そうすると必然的に「チーム１厳しく」なります。
また、わからないことだらけなので隅々までコードを見ます。（知識があるとかえって飛ばし読みとかしますよね）
そのおかげで、他の誰も気付いていないケアレスミスに気づくことも多々ありました。結果的にコード品質にも寄与できます。
1-2. 新人がレビューすることが迷惑でないかという「安全性」の障壁
「新人が成長することがチームのためになる」、LITALICOの優しいマネージャー陣はそうおっしゃってくださいますが、とはいっても新人が成長過程で、強い先輩よりも低い生産性でタスクをこなしていたら、短期的には迷惑かけちゃうじゃん、、、、と自分は思っていました。
コードレビューを通して学ぼう！、と今自分が言ったわけですが、当初の自分はこういった懸念が拭えませんでした
ここについては、段階を踏むということ、そして感謝することで解消しました。
まず当時メンターだった方とマネージャーに相談し、以下の方針でPRレビューに取り組むことにしました。
- まずはリリース優先度が低いPRからレビューに挑戦する
- 自分はサブのレビュワーと位置付け、自分とは別にメインのレビュワー２名をアサインしていただく
自分のような新人は先輩のコードを読むのが遅くなり、リリースに悪影響を与える懸念があったため、上記のようにまずはハードルをさげて取り組み始めました。
そこから段階的に自立し、いまは、一人のレビュワーとして、幅広くいろんなPRを見ることができるようになりました。
リリースにかかる時間以外でも、自分がどうでもいいことを指摘してしまうのではないかという懸念もありました。これに関しては、レビュー自体の精度をあげる努力をすることで、段階的に有用なレビューをできるように心がけました。レビュー精度の向上については#3. 具体的なプロセスを参照ください。
また、新人からの指摘は、どうしても「生意気感」が付随しちゃうので、その分、自分の質問、指摘に答えてくださった方にはちゃんと感謝しました！ 自分の学びのためにリソースを割いてくださっているのもありますし、、、。
このまえ、エンジニアチーム内の振り返り会(チームメンバー同士のFB会)で、よく自分の質問に答えてくださるベテランエンジニアに対して、「いつも丁寧にPRレビューに答えてくださりありがたい」とFBさせていただいたところ、「自分の実装の背景を言語化できるから、質問はむしろありがたい」とおっしゃってくださいました。
こうしたコミュニケーションをとると、より「安全に」、「厳しいレビュワー」になれると思います
2. それが価値があることなのか
さて、新人だからこそ厳しいレビューが可能らしいが、それをやる意味があるのか？とお思いのそこのあなた
あります！！
ここに関しては大きく２つの価値があると思っています
2-1. 実装の選択肢が増える（個人の成長）
2-2. 意外と、誰も気付いていないことに気付けたりする（チームへの寄与）
2-1. 実装の選択肢が増える
PRレビューによって、先輩のコードを参考にできるのは言うまでもないですが、「チーム１厳しく」やることで、細かな部分も参考にできます。
先日も、ある単一の画面において、特定のフローでアクセスした時のみ、部分的に画面表示を変える、という仕様の実装をしていたのですが、
自分の発想で思いついた実装方針だと、挙動が要求仕様とやや差分があり微妙でした。
しかし、ふと、先輩のPR見た時に似たようなシチュエーションがあったぞと思い至り、そのPRを検索して見つけたものを参考にするとうまくいきました！
先輩のその実装は、そのPR全体においては枝葉末節であり、レビュー時に隅々まで見ていなければ、見つけられなかったと思います。
わからないところだらけであるからこそ、細かな箇所まで学びになるはずです。PRをしゃぶりつくしましょう
2-2. 意外と、誰も気付いていないことに気付けたりする
厳しいレビューをするなかで、不明点をなくすために細かく実装を追っていると、この変数名わかりづらいな、ここの処理どういう意図なんだろう、など細かな点で気づくことがあります。
先輩たちは慣れているがゆえに、かえって細かな点で気づかないことがあるので、これは、ピュアにレビューできる新人が「厳しく」みることの利点だと思っています。
上の例だと、実装を追う時に「186px」って何の数字なんだろうと疑問に思ったのがきっかけです。
コードの可読性があがれば、メンテコストもさがり、結果的にチームに寄与することができます。
3. 具体的なプロセス
さてここで、新人が厳しいレビューをする具体的なプロセスを共有させていただきます！
- 実際にレビューをやってみる
- 仕様を満たしているか挙動確認
- 実装の確認
- 不明点洗い出し
- 不明点の中で、知識不足に由来するものは調べる
- 調べてもわからないもの、実装者の意図がわからないものはどんどん聞く
- レビューをとおして得た知識を復習、よりよいレビューのために勉強
これをループさせる形で、いまもやっています！
２について、まず何がわからないのか、何を調べたらわかりそうかを整理しましょう。わからないことが複数ある状態で無計画に調査を始めると、いま自分が何を調べているのかすら迷子になることがあるので、、、
（実体験）
自力で調べたらわかるのかどうかすらわからない場合は、そのまま質問しましょう。その場合、すぐに答えを聞かずに、「何を調べたらここについてわかるのか」だけを聞くのもありです！
あとはひたすらに指摘、質問です。「全部見てやる！」という誇りをもってやりましょう！
忘れてはいけないのは、学習としてレビューをしつつも、指摘の精度をあげる努力も並行すべきということです。学習しつつも、よい指摘をできたらいいですよね。
自分の場合は、レビューや実装、勉強を通して学んだことを、「レビュー観点」というスプシにまとめ、レビューの際に活用していました。以下はほんの一例です
レビュー観点で参考にしたのは、今までのレビュー、実装の経験によるものと、以下の書籍が大きいです。
リーダブルコード
超絶わかるソフトウェア設計入門
まだまだ、ここに関しては成長の余地があるので、これからも精度を高めていきたいです！
4. どういう環境だと、「新人が先輩のコードを厳しくレビューする」をやりやすいのか
最後に環境面についてお話ししたいです。
偉そうに、「新人こそ、チーム１厳しいレビューが可能であり、それは有意義なのだ！」と述べてきたわけですが、そもそも環境がそれを許してくれるかはやはり重要です。今振り返ると、チームの優しい先輩のおかげだなとつくづく感じます。
端的に言うと、以下２点が環境面で要点かなと思います。
- 意見を一旦聞き入れてくれること
- 感謝を伝える機会があること
1つ目について。新人がなんらかの指摘をしたときに、「逆にそれ意味あるの？」で突っぱねられると、学びになかなかできないです。完璧な根拠がないと意見してはいけない、は新人にはハードルが高すぎます。（もちろん根拠を用意しようとする新人側の努力も大事）。
根拠が時に貧弱なこちらの意見を聞いてくれた上で、説明いただけたことが大きいなと思います。
2つ目についてですが、1-2. 新人がレビューすることが迷惑でないかという安全性の障壁の項目の後半で書いたように、普段レビューに答えてくださる先輩に改めて感謝できる場があることが重要だなと思います。特にリモートワークが多いエンジニアチームだと、なかなか改めて感謝を伝える機会がないので、定期的にお互いのFBをする場は大事だなと実感しています。
相手が、自分の学びに付き合わされていることを「許してくれている」という心理的安全性が確保できたことも、大きかったと今振り返って思っています。
さいごに
上半期、結構力をいれていたPRレビューについて、改めて振り返る良い機会になったなと感じています！
ひとつの成長の事例として、誰かの役に立てたら嬉しいです。
僕自身、ネガティブな性格もあって、入社当初は不安で不安で仕方なかったのですが、わからないことをなくすために何かに徹底して取り組むということができたのは一つの自信にもなりました。
今回、勇気を出してアドカレに初参戦したわけですが、個人的には、経験豊富なベテランの意見も参考になりますが、不安だらけの新人こそ、矮小な事例でもいいから、お互いの努力と成長をどんどん共有すべきと考えています。
というわけで、同期(23卒)や後輩がアドカレ後半でたくさん記事を書いてくれることを願っています
明日は、@eito_katagiri-litalico さんです！
それでは〜