プロジェクトリーダーとしてコードレビューをどう進めているか

本記事は【Advent Calendar 2025】 6日目の記事です。
🌟🎄 5日目 ▶▶ 本記事 ▶▶ 7日目 🎅🎁

こんにちは、上村です。
私は2年半ほど前からプロジェクトリーダーとしてシステム開発管理をしています。
その中で、コードの品質を担保するためにコードレビューも実施しています。
ただ、最初はどうレビューしたらいいのか分からず、以下を行いながら手探りで進めていました。

他プロジェクトのレビュワーに観点を伺う
レビューの方法や観点などWebで読み漁る

コードレビューを行ううちに少しずつ進め方が分かってきましたが、他の方のレビューも見ていると「そういった観点もあるのか！」と今も感じることもあります。

また、今は以下のフローでレビューを進めているため、開発メンバーにもレビューをアサインしています。

レビュイーがプルリクエストを提出
【1次レビュー】開発メンバーのレビュー
【2次レビュー】プロジェクトリーダー（上村）のレビュー

その中で、レビューをどう進めればいいか悩んでいそうな雰囲気を開発メンバーから感じることもあります。
そういった経緯もあり、レビューの進め方のヒントになればと思い、この記事を書いてみました。

※あくまで私見を整理した内容です。

自分がレビューするときのスタンス
コードレビューでの確認観点
まとめ

自分がレビューするときのスタンス

理由とセットで指摘する

ロジックの修正などコアな部分を指摘するときは、可能な限り理由も示すようにしています。
※コメントコードの誤字の修正といった軽微な内容であれば、修正してほしい内容のみを指摘することも多いですが。

というのも、修正してほしい内容だけ聞いてもレビュイーは納得感を持てないこともあると思います。
個人的には、なぜ修正が必要かをレビュイーにも納得してもらったうえで修正してほしいと考えています。
※過去に、「理由もセット🍔🍟🧃で指摘してくれるので納得感を持って受け入れることができる」という声もいただきました。

修正案は“選択肢”として提示する

明確な正解がない修正案に関しては、「このような修正方針はどうでしょうか」といったスタンスでレビューコメントを入れることもあります。
※明らかに仕様に反しているといった箇所であれば、ストレートに「修正をお願いします」と指摘することも多いですが。

例えば変数名をより分かりやすくしたいというケースについて、メンバーによって分かりやすい変数名が微妙に異なるケースもあるかと思います。
このような場合、「こういった変数名はどうでしょうか」とレビューコメントを入れます。
もしレビュイーとしても納得感のあるものであれば、その通り修正してもらうケースもあります。

一方、レビュイーが他に良い案を持っていれば、それを私に提案してもらいます。
そちらのほうが分かりやすいと感じれば、その内容で修正してもらうこともあります。

レビュイーと協力して良いコードを作り上げたい想いから、このようなスタンスで進めています。

意図を確認する

コードレビューしていると、「なぜこのような修正をしたのか」と感じることもあります。
意図があるケースも多いですが、そこをレビュワーがスルーすると、今後思わぬ不具合に繋がる可能性もあります。
そこで、私は些細な修正内容に対しても、気になった点があればその修正意図を質問するようにしています。

例えば、以下のようなマジックナンバー「60」を扱うコードがあるとします。

    /**
     * スコアに応じて合格/不合格を返す
     * @param score 試験の得点
     * @return true(合格) または false(不合格)
     */
    public static boolean isPassed(int score) {
        if (score >= 60) {
            return true;
        } else {
            return false;
        }
    }

おそらく「60」は合格点と表しているんだろうなと読み取れるのですが、認識が合っているかと質問します。
そのうえで、合格点の定義ということが伝わるよう、以下の通りマジックナンバーを使わないコードに修正してもらうというのが具体例として挙げられるでしょうか。

    // 合格点の定義
    private static final int PASSING_SCORE = 60;

    /**
     * スコアに応じて合格/不合格を返す
     * @param score 試験の得点
     * @return true(合格) または false(不合格)
     */
    public static boolean isPassed(int score) {
        if (score >= PASSING_SCORE) {
            return true;
        } else {
            return false;
        }
    }

必要に応じて口頭レビューを行う

修正内容によっては、設計が大きく変わるケースもあります。
その場合、コードを読むだけでは処理の理解が困難なこともあります。

そこで、口頭レビューの機会も設けます。
レビュイーに全体像から説明してもらい、どういう理由で設計変更を入れたのかを認識合わせしていきます。
レビュイーからの説明の中で、気になる点があればレビュワーからコメントを入れていきます。
口頭の方が情報多くやり取りできるので、格段に理解が深まりやすいです。

ただし、口頭での会話のみだと記録が残らないので、会話内容（設計変更趣旨・レビュー結果）も文章で残します。

良いところはリアクションする

淡々とレビューコメントを入れていくと、ドライなやり取りになりがちです。
少しでも前向きな空気を作りたい・良い点は継続してほしいという想いから、以下も実施しています。

良い修正点は、「[good]◯◯が良いと思いました！✨️ 」というレビューコメントを入れる。
開発メンバーのレビューコメントが良いと感じたら、リアクションスタンプ👍️をつける。

コードレビューでの確認観点

仕様通りのコードとなっているか

顧客要望をもとに私の方で仕様を検討し、開発メンバーに実装を進めてもらうのが私の開発管理業務の中心となります。
そこで、仕様通りのコードとなっているかは必ずチェックします。
細かい仕様の開発が漏れていたというケースもあるので、重点的に確認します。

意図が伝わるよう情報を残せているか

私は2年半ほど同じシステムを担当しています。
2年半と聞くと長い期間担当しているように感じるかもしれませんが、未だ既存コードの意図を読み解けないケースもまだまだあります。

そこで、意図があって特殊なコードの書き方が必要になる場合、その意図をコードコメントとして残してもらうようにしています。
そうすることで、将来的にメンバーが入れ替わったときも、新規メンバーが該当コードを読み解きやすくなります。

例えば、以下の通りユーザーの状態をログ出力してから呼び出し元に返すメソッドがあるとします。

public int calculate(User user) {
    int result;

    if (user == null) {
        result = 0;
    } else if (!user.isActive()) {
        result = -1;
    } else {
        result = user.getScore();
    }

    log.debug("result={}", result);
    return result;
}

よりコードをスッキリさせるため、以下の通り分岐ごとに即座にreturnさせるという書き方もあります。
しかし、分岐ごとにログ出力処理も入れないといけないため、実装漏れが発生するリスクもあります。

public int calculate(User user) {
    int result;

    if (user == null) {
        log.debug("result=0"); 
        return 0;
    } else if (!user.isActive()) {
        log.debug("result=-1"); 
        return -1;
    } else {
        log.debug("result={}", result); 
        return user.getScore();
    }
}

そのため、「 // ← どんな分岐でもここで1回だけログを残したい」というコードコメントを残しておくと、return処理を1箇所のみに記載した意図が伝わりやすくなります。

    log.debug("result={}", result); // ← どんな分岐でもここで1回だけログを残したい

※このサンプルコードは簡単なものなので、コードコメントがなくても意図が類推できるかもしれません。
しかし、実際の業務で扱うコードは複雑なものも多いので、コードコメントがないと意図が読み取れないケースも多いです。

(テストコードを修正した場合)妥当な検証となっているか

私の担当システムでは、テストコードも整備しています。
そのため、プロダクトコード・テストコードをセットでコードレビューするケースが多いです。
私は以下理由により、プロダクトコードほどは細かくレビューしません。

テストコードは修正量が数百行～千行単位と、プロダクトコードに比べて圧倒的に多いケースがほとんど
ユーザーに直接的な影響は発生しない
※プロダクトコードの場合は、修正内容にミスがあれば不具合に繋がるため、毎回しっかりレビューしています。

しかし、意義のある検証ができているかは抑えたいため、以下を中心にレビューするようにしています。

処理したデータが一通り検証できているか
カバレッジ網羅できているか

カバレッジ網羅については、例えばJaCoCo*1を利用することで機械的に確認できます。
そのため、特に「処理したデータが一通り検証できているか」についてもう少し例を挙げて説明します。

例えば、以下メソッドがあるとします。

    /**
     * 2つの整数について
     * ・合計値
     * ・差分値（a - b または b - a）
     * を返すメソッド
     */
    public ResultPair calculatePair(int a, int b) {
        int sum = a + b;
        int diff;

        // a >= b なら a - b, そうでなければ b - a
        if (a >= b) {
            diff = a - b;
        } else {
            diff = b - a;
        }

        return new ResultPair(sum, diff);
    }

calculatePair()では、sumとdiffを計算して返しているので、この2つが期待値通りかは検証したいです。

ここで、calculatePair()に対するテストコードとして、以下プルリクエストが提出されたとします。

public class CalculatorTest {

    @Test
    void testCalculatePair_aGreater() {
        Calculator calculator = new Calculator();
        ResultPair result = calculator.calculatePair(10, 3);

        assertEquals(13, result.getSum(), "sum should be 13");
    }
...
}

このテストコードではsumの検証はできていますが、diffが検証できていないので、「diffの検証処理を追加してください」というレビューコメントを入れます。