はじめに
この記事は🎅GMOペパボエンジニア Advent Calendar 2022 12日目の記事です。
さまざまなエンジニアのメンバーが楽しい記事を書いているので、ぜひご覧ください。
初めまして。ほりゆうと申します。
2022年の7月に物流企業からGMOペパボ株式会社に転職したエンジニアです。前職ではLaravelやSalesforceを用いて倉庫を管理するSaaSを開発しておりました。現在はSUZURIというオリジナルグッズ・アイテムの作成・販売ができるWebアプリケーションの開発をしております。
以前から憧れだった会社・メンバーの皆様と一緒にこうしてAdvent Calendarに参加できていて光栄です。ついこの前までは志望者としてみなさんの記事を読んでいました。この記事が、そんな誰かのためになれば幸いです。
本記事では、転職前に自分が気になっていたコードレビューについてまとめたいと思います。
前職もとても良い環境でしたが、コードレビューの機会は少なかったので、研修や現職でどんな感じでコードレビューしていただいているのか、よくレビューをいただくポイントや印象に残っているもの、またレビューしてもらう立場としてどう準備しておくとよいかについてまとめたいと思います。
レビュー内容
印象に残っているレビュー内容で共有できるものをまとめました。
🎁 論理演算子系
- if hoge
- raise ActiveRecord::RecordNotFound if hoge != fuga
- end
+ raise ActiveRecord::RecordNotFound if hoge && hoge != fuga
論理演算子は特に短く書く方法や、条件の評価順によって書かなくていいものなどのアドバイスをいただきました。
🎅 意味の多様性
◯◯◯という言葉には、◯◯◯という意味と●●●という意味があるので、より正しい意味が伝わるようにしましょう。
実装しているコードのテストや、ユーザーが利用する画面に出力されている文字などに関して、自分が受け取ってほしい意味とは別の意味に取られかねない表現を指摘していただくことがありました。
言葉に関しては、すでに実装段階でPMやディレクターの皆さんとお話しして決まっているものがほとんどでしたが、改めて実装した後に、プロジェクト以外のエンジニアの方からレビューをもらうと「そもそもこの表現って正しいんだっけ...？」という見直しの機会になりました。
🎄 実行されるクエリや実行計画を見て、重くならないかチェック
実装で特定のデータを取得してくる際にORMなどを使用する場合、実行されるSQLやその実行計画をチェックするようになりました。
🎂 メソッドやコンポーネントをまとめること
共通化をできる処理はまとめたり、Reactの実装では再利用できるような形でコンポーネントを作成しておくなど、細かくまとめることを指摘していただき、意識するようになりました。
再利用できるものは自分の後に実装するメンバーの負担を減らすことにもつながるので、積極的に取り組んでいきたいです。
入社後、たくさんのレビューをいただきましたが、以前より特に意識するようになったものを紹介させていただきました。
次に、レビューをしていただく中で、レビュー前にやっておいた方が良いなと思ったものを紹介します。
事前に共有しておくと良さそうなこと
🍗 影響範囲の調査と共有
実装によって予期しない場所に影響を及ぼしてバグを産まないかを調査し、影響する可能性がある場所をリストアップしておきます。事前に絶対に影響するところがわかると、レビュワーの方も実装者が知らない他に影響しそうな場所を推測できるためです。
🍷 セルフレビュー
コミットする前や、レビューを依頼する前にdiffを見てセルフレビューしてみます。また、コミットメッセージには実装した内容そのものではなく、実装した意図を書いておきます。自分で気付ける限界まで調査し、その上でレビューをもらうと良いと思います。
@jnchito さんの記事がおすすめです。
プログラミング初心者はgit commitする前に必ずdiffを自分でレビューするクセを付けよう - give IT a try
🎉 レビューポイント（見てもらいたいところ・不安なところ）
特にレビューをしてほしいポイントを書きます。全部見てほしい気持ちもあると思いますが、そういった場合は不安な場所を書くと良いと思っています。レビュワーも注意すべきところが絞られるので、処理集中してレビューできると思います。
💝 動作確認方法の補足
コードだけではわからない動作確認方法を捕捉したり、コードを見れば分かるがアクセスしてほしいリンク、実行してほしいコマンドを添えるなどの配慮があった方がレビュワーのストレスにならないと思います。
🍾 実装の背景
この機能をなぜ実装したのか、関連のissueやそのissueを踏まえてなぜこのような実装になっているのかなどを説明します。また、プルリクエストを小分けする場合は、実装の全体像と現行のPRがどこに当たるのかを説明するのも大切だと思います。
実装の背景や全体像がわかると、レビュワーは前提を踏まえてより良いアドバイスができるためです。
以上が、レビューをしていただく中で自分が事前に注意しようと思ったところです。見ていただくことを感謝して、なるべくストレスがないように気をつけていきたいと思います。
まとめ
入社後にたくさんのレビューをいただいて、気づいたことをまとめてみました。自分もレビューする機会がたくさんあるので、これからも他のメンバーの役に立てるように、そしてGMOペパボの「いるだけで成長できる環境」づくりに貢献できるように頑張っていきたいと思います。
レビューサイコー！