チーム開発におけるコードレビュー — 品質ライン、または幅

ソフトウェア開発のチーム開発におけるにおけるコードレビューは、単なる確認・承認作業ではなく、チーム内で設計や実装のあり方に関する重要なコミュニケーションの場でもあります。

またチームのリードから見るとコードレビューはその主要な関心事の一つである品質に関わる重要イベントでしょう。

コードレビューはチームの中の複数のエンジニアが関わるタイミングでもあるので、衝突がおこりやすい場面でもあります。コメントが100近くついてしまい収集がつかなくなったコードレビュー、レビュアーがコメントを返さなくなっていつまでもマージされる気配がないプルリクエスト、そんな経験を持つソフトウェアエンジニアも多いのでないでしょうか?

ここではコードレビューを中心にチーム開発においてコード品質を保つことの難しさの原因、それに対する自分なりの考え・実践について書きます。

なお、私は品質を落としたとしても開発スピードが上がらないというTakuto Wadaさんの質とスピードに関する考え、特に「技術力のある人はある程度急いで作ったとしても一定以上の品質のコードを書くし、意図的に品質を落としたとしても速度はあまり上がらない。」の考え方は非常に重要と考えます。この記事ではその上で「ではプロダクトごとの目指すべき品質をどう設定し、どのようにチームに定着させていくのか」について考えることを目的とします。

複雑さ — なぜ難しいのか?

チーム開発でのコードレビューはどこに難しさがあるのでしょうか? 2点考えられます。

個人の作業の成果物をチームに移す難しさ

1点目の難しさはチーム開発そのものの難しさです。

ソフトウェアを書く作業は通常、個人で行います。作業の粒度にもよるでしょうが、数時間-数日は個人で作業を進めることになります。その後プルリクエストを作成し、コードレビューを行います。コードレビューにおけるコミュニケーションの難しさの理由の一つは個人の作業の成果物をチームに移す難しさ”と言えるかもしれません。

考えてみればこれはチームでの仕事の進め方として特徴のある状況ではないでしょうか。例えばレストランのキッチンであれば料理人が皆同じ時間と場所で作業します。他の料理人の状況も見えますし、何らかの問題があってもフィードバックをリアルタイムに行うことも可能でしょう。それに比べるとソフトウェア開発の個人での作業は長い傾向にありますし、フィードバックを受けるまでのリードタイムも長い状態です。

ペアプログラミング・モブプログラミングや早期にドラフト・WIP(Work In Progress)状態のプルリクエストを奨励する運用はこの課題を解決するための実践方法だと言えます。

スキルがポータブルであることからくる多様なバックグラウンド

2点目の難しさは人・コミュニケーションに関わる部分です。

プログラミング言語やGitHubなどのツール、クラウドインフラの環境など、ソフトウェアエンジニアの開発に関わるスキルは組織を問わずもち回せるポータブルスキルが多いことから、人材の流動性が高いことはソフトウェアエンジニアの特徴の一つと言えると思います。

大学教育・独学・スクールなどスキルを最初に身に着けた場所も様々で、様々なバックグラウンドの人がソフトウェアエンジニアの業界には多いように感じます。これは人材の多様性に繋がっている点として私はポジティブにとらえていて、自分自身がソフトウェアエンジニアとしての仕事を長く楽しめているのも、集まる人の多様性から来ている部分があります。

チームメンバーの多様性はソフトウェア開発の現場でも良い影響を与えます。私自身の経験でいえば「ここはこういう設計指針を考えている」と話した上でメンバーが実装をした結果「使っているライブラリのドキュメントにもっといい方法が書いてあったのでそうしたよ」とフォローされることがままあります。

これは自分自身が比較的全体を大まかに把握して早く手を動かし始める事を好む傾向にあり、チームメンバーがドキュメントをきっちり読み込んだ上で手を動かすタイプであったことから来るチームの多様性が良い方向に運んだ例といえます。

一方で多様性がネガティブに働く場合も当然あります。各メンバーの過去の経験はチームににってお貴重な資産ですが、以下のようなやりとりになってしまうと話は平行線です。

Aさん「過去のチームではもっと○○しなければいけなかった」

Bさん「いや、自分は○○しなくても十分だった、○○は不要だ」

ここで現在のチームの指針が示さなければチームの中に軋轢が生まれ、多様性はチームの生産性・コード品質の足かせになっていくでしょう。

問い

このチーム開発における品質についての難しさに対して2つの問いをたてました。

Q1:「これにそっておけばOK」のドキュメント・ルールを作ることは可能か?

人によってコードのスタイルや品質に関する基準がバラけるとすれば、最初に思いつくソリューションは「これにそっておけばOK」という統一的なドキュメント・ルールを作ることでしょう。それは可能でしょうか?

Q2: 品質についてチームでどうコミュニケーションをするのか?

ドキュメント・ルールを整備した上でもチーム開発を行う上では必ずコミュニケーションはついてまわります。品質についてどのようにチームでコミュニケーションすることで軋轢を避け、チームの生産性・コード品質を維持していくことができるでしょうか?

答え

Q1:「これにそっておけばOK」のドキュメント・ルールを作ることは可能か?

A1: ドキュメント・ルールでカバーできる範囲とできない範囲があるので全ては難しい。できる部分はコード化して自動実行する

煮え切らない答えですが、ドキュメント・ルールでカバーできる範囲とそうでない範囲が存在します。

ドキュメントやルールでカバーできる範囲としてコードの静的解析・テストカバレッジなどの定数的指標といったコード化可能な部分について考えます。

ここでドキュメント・ルールは「○○に関しては○○を必ず行う」というような強い制約になるものを想定しており、指針を示すようなドキュメントに関してはQ2で扱います。

コードの静的解析

これは既にチームで実践されていることも多いと思いますが、インデントやコードフォーマット、循環的複雑度など静的解析で検知可能なものはRubyであればrubocop、JavaScriptであればESlintなどのlintツールでチェックし、CIでfailするようにしておけば、チーム内での標準とすることができるでしょう。

これはある種のコード化して自動実行可能なドキュメント・ルールであると言えます。逆にここで見きれないものは人によるコードレビューの対象になっていきます。

ただ当然lintのルールをチームで合意していくためのディスカッションが必要になりますし、ルールは状況の変化に応じて見直し・メンテナンスも必要です。そういった意味ではQ2の品質についてのコミュニケーションに通じる問題は引き続き残ります。

テストカバレッジなどの定数的指標

ある程度運用が進んだプロダクトであれば、自動化されたテストのカバレッジなどのより直接的な品質の定数的指標を定めることも可能でしょう。○○レイヤー以下では○○%以上のテストカバレッジを維持し、下回った場合リリースできない、というようなルールです。

定数的な数値指標は見えやすく扱いやすく、運用しやすいものです。一方で経験上、プロダクトの初期(リリース2年くらいまで)ではまだプロダクト自体の仮説が多く、頻繁に根本的な部分で仕様の変更が起こります。コードベースは安定しない状態になり、定数的な指標を置くのが難しいと感じます。定数的指標を置くタイミングと運用に関してはプロダクトのフェーズの見極めと併せて検討することになるでしょう。

コード化して実行できる範囲を増やしていきたい

現状コードを機械的にチェック可能な部分は決して多くないと感じます。しかし、GitHubのCopilotのようにその可能性を拡大してくれる存在は登場しています。コードとして記述可能でCIで自動チェック可能な範囲はこれから今以上に拡大していくと思われます。

ドキュメント・ルール化することを目的としない

ドキュメント・ルール化はチームのアウトプットの生産性・品質を高めるための手段であり、目的としない、というのは重要なポイントです。

ルールを増やしていった結果、新規メンバーの参入が難しくなりい、結果ルールは存在するものの誰にも守られていない、という状態になるのは本末転倒です。

Q2: 品質についてチームでどうコミュニケーションをするのか?

上記でシステム的にチェックできないものが人によるコードレビューの対象になります。明文化し機械的にチェックできないものが対象となるので必ずソフトウェアエンジニア同士のコミュニケーションが発生することになります。

実際的なチームでの品質に関するコミュニケーションの考え方と私が行っている実践をいくつか紹介します。

ペアプログラミング・モブプログラミング

まず1つ目はコードレビュー以前のコミュニケーションとしてペアプログラミング・モブプログラミングを実施することです。

冒頭でふれた100近くのレビューコメントがつくコードレビューの例の原因について考えてみると、その原因の一つとして、コードを書く以前の設計やこのPRにおけるゴールやスコープ、またその前提となるドメイン知識の共有など、コードそのもの以前で存在していた認識のズレがコードレビューの段階で発露したことが挙げられます。

その対策の一つとしてペアプログラミング・モブプログラミングは有効であると言えます。これらの取り組みはコードを書く作業とレビューをリアルタイムに行うことがその特徴の一つです。コードを書き上げレビューに入る以前に多くの認識のズレを取り除くことができます。

ソフトウェアエンジニア複数人の時間を同期してコストのかかる実践ですので、実施するタイミングは検討が必要ですが、有力な方法論の一つとして使える状態にしておきたいです。

品質を線ではなく幅で見る

私がテックリードとしてあるチームのコードベースの品質について考える時に、コードの品質を線(クオリティライン)ではなく幅で考えて、チーム内に共有できるように心がけています。

幅の設定とチームへの定着

コードの品質を100点満点で表すとすれば、70–80点の間をチームでの品質として許容できる幅として設定し、チームに共通認識として持てるように働きかけるイメージです。

ソフトウェアエンジニアの多くが自分のコードのスタイルや「こういったコードが良いコードである」という考え方を持っていることでしょう。設計の原則やアプリケーションのドメイン固有の知識、細かなコードのスタイルに至るまでその対象は多岐に渡ります。

複数人のソフトウェアエンジニアがチームになって取り組みに当たり、大なり小なりメンバー間でギャップが起こるのは必然です。そこに対して「これが正解」という強固な一定のラインを設けることはチームの多様性を下げ、かえって生産性を落とすことに繋がると考えます。

提出されたコードが一定の幅よりも低い状態であれば、コードレビューの中で具体的なポイントを示します。一般的に1開発チームは多くとも8–10人までの範囲に留まるので、コードレビューを通じて品質の幅を徐々にチーム内で統一していくことができます。

逆に設定した一定の幅よりも提出されたコードの品質が高い状態もありえます。既存コードのリファクタリングの範囲が過剰であったり、レイヤリングの変更など根本的なアーキテクチャに関わる変更が入る場合などが想像しやすいでしょう。この場合テックリードは明確にやらない事を示し、その理由をメンバーに説明できる必要があります。

これらのチームの中での働きを通じて設定した品質の幅はチームに定着していきます。

品質はプロダクトやフェーズによって変化する

設定する品質の幅はプロダクトやフェーズによって変化することも重要なポイントです。

プロダクトによって想定されるリリースのスピード、コードに求められる変化の量が違い、それに応じてユニットテストの量やアーキテクチャの複雑度とメンテナンスのしやすさのトレードオフなどが発生します。

また同じプロダクトでもクローズドなβ版を提供している段階と商用開始した段階、スケールして多くの顧客がついている状態ではコードの品質に求められるものも変わります。適切にフェーズを見直し、設計の指針やリアーキテクチャリングのタイミングを見極める必要があります。

コードレビューでの実践

チームに品質の幅を定着させていくために私が日々のコードレビューの中で実践している方法について紹介します。

GitHubなどでのコードレビュー時に、コメントの先頭に4つのレベルを明記することを実践しています。

request change

バグや品質上の問題があり、変更が必要だと考えている部分

suggestion

改善が可能だが、品質の幅の範囲内であり変更は必須ではないと考えている部分

question

コードの意図が読み取れず、PRを出したソフトウェアエンジニアに質問が必要な部分

comment/memo

コードを読む中での自分用のメモ書き、自分自身の認識を書き残しておく部分

まずレビューイにはすべてのコメントについて目を通してもらうようにお願いします。

request change/questionに関してはコードの変更、またはコメントの返答を期待しますが、suggestionやcomment・memoに関しては一読して問題なければ返答してもらう必要はありません。

request change/questionが解消した時点でPRをApproveします。suggestionやcomment・memoのみをつけたPRには最初からApproveをします。

品質の幅を設定し、チームに定着させるためにsuggestionの運用が特に重要だと考えています。

必ず変更を入れておきたい箇所を限定し、マージまでの過剰なやりとりを減らすことを心がけています。逆に最低限のチェック項目だけを機械的に確認していくようなレビューでは、気になってはいるもののスルーされるような項目に関しても意見は表明し、チーム内で話題にはのぼるようにしておきます。

設計に関するドキュメントを残す

これは自分自身も十分に実践できておらず、これから取り組みたいと思っているポイントです。ここでのドキュメントは先に触れた 「これにそっておけばOK」のドキュメント・ルール とは異なり、ある程度の大きさの実装やアーキテクチャの変更を伴う箇所に対しての記録としてのドキュメントです。

所謂Design Docを書くのが良いのではないかと考えています、が、実際のフォーマットや運用に関しては実践の中で良い形を見つけていきたいと思います。

これまでもスクラムのイベントの中に入れ込む形でチーム内に設計やアーキテクチャリングの共有やディスカッションは行ってきたのですが、チームメンバーの入れ替わりや時間の経過、さらにチーム外への共有も考えるとドキュメントが残ることの価値は大きいと考えます。

まとめ

チーム開発においてソフトウェアの品質を保っていくことの難しさ、特にコードレビューの難しさに対する自分なりの実践について書きました。

現状はコードの静的解析や定量的な数値目標を設定し自動で判定できる部分をキープし、そこで判定できない部分に関しては品質の幅をチーム内で持てるようにコミュニケーションしています。

自動で判定できる部分はこれからもその範囲を広げていけると考えます。現状チーム内のリアルタイムのコミュニケーションに寄っている状況もDesign Docなどのドキュメントの強化によってよりよく改善できると考えています。