ykrods note
(Update: )

Webシステム開発でのコードレビューより前のレビュー

レビューについて思うことのメモ

論旨

Web開発にて、レビューはコードだけでなくその前の工程もレビューするべきではないか

具体的な対象としては以下が挙げられる

  • 要件定義

  • 見積もり・スケジュール

  • アサイン

  • 設計

  • 開発の進め方

特に、計画に無理があれば品質よりスケジュールが優先される以上、コードレビューの効率を上げるより見積もりの精度を上げる方が重要度が高いのでは

レビュー範囲を広げるべき理由

  • コードレビューの観点として仕様を満たしているかというのがあるが、仕様観点でレビューするためには仕様が必要

  • 仕様を満たしていても、仕様が要件を満たしているかは別の話なので、連鎖的に要件も必要

  • 上にも書いたが計画に無理があれば品質よりスケジュールが優先されるので、コードレビューをするためにはする余裕のあるスケジュール・アサインを組むことが必要

コードレビューだけが浸透した経緯

そもそも 1028-2008 - IEEE Standard for Software Reviews and Audits でもコードのレビューは5形式あるレビューの中の一部で実施されるもので 1 ソフトウェア開発の文脈ではレビューはドキュメントや管理体制などより広い範囲を扱う 。ではなぜWebの開発においてコードレビューだけが浸透しているのかというと、レビュー文化が浸透した経緯にあると思われる。

現状Webのシステム開発で行われているコードレビューは、OSS由来だと思われる。

  • OSS活動も活発なできる系のエンジニアがコードレビュー当然やるよね?という感じで導入したのが一般に広まった、という流れではないかと思われる。

  • Webというのは概ねプログラミング技術があっても格式張った会社が苦手な人間(簡単にいえばスーツを着たくないような人種)が入る業界なので、厳格なレビュープロセスを実施するような会社からWebに移ってきた人というのは少数ではないかと思われる。

    • (今40代前後の方はそういうキャリアパスを辿っている可能性があるが実際のところどうなのだろう。システム開発会社でもレビューを厳密にやっているのは一部の大手企業だけなのか、一般的にレビューが浸透しているのか

  • OSSにおいても、基本的な設計や思想、ソフトウェアの方向性についてディスカッションが行われることはあり、それはパブリックなレビューと捉えることもできるが、それをシステム開発で行うことはあまりないように思う(プロマネの役割として、プロマネが決める)。

システム開発でのコードレビュー

OSSの開発モデルを、そのまま社内に持ち込むのは止めたほうがいい(もしくはコードレビューの話) - kazuhoのメモ置き場 で述べられて通り、OSSの開発モデルはバザール 2 といわれるもので、いくつもある Pull Request から最良のものを選ぶ(採用できないものを捨てられる)が、仕事では通常そうはいかない。コーディングにかけられる時間は決まっている。

時間が決まっている以上、レビュー段階で初めて問題が発覚しても遅いと言える。システム開発におけるコードレビューはテストという最終工程の一個前の作業であり、コードをブラッシュアップすることや見落としのチェックを行う以上の指摘をこの段階で行うものではない。

つまり、 もしあなたがレビュワーで、「このPRは出来損ないだ、マージできないよ(山岡風)」とか「そもそもこのコードで何がしたいのか分からない」という状況になったことがあるならば、それはレビュイーではなく、コードを書く前の開発工程に問題がある

時間が限られる前提での推奨事項

  • レビューの単位は大きくても3人日分程度の実装に収めるべき。

    • それ以上は手戻りになった場合の影響が大きすぎる

    • 差分が大きいとレビューする方も辛い

  • レビューのためにタスクを分割するのであれば、分割されたうちのどの範囲を実装するのか、進め方についてレビュワーと事前に合意しておくべき

    • 進め方についてはプロマネが決めることでは?という意見もあるかもしれないが、プロマネにタスクが集中するとボトルネックになるので、分散できる仕事は分散した方が良い

  • 設計段階で外していると手戻りが大きいので、設計レビューで確かめるべき

    • 設計レビューとコードレビューを同じ人が行えば、効率よくレビューができる。

    • 設計の妥当性は仕様との比較から判断するので、仕様書が必要

  • そもそも仕様をドキュメントにまとめるべき

    • もし全レビューをプロマネがやっているとしたら、それは仕様がプロマネの頭の中にしかないのが原因

    • この体制で回せるのは最大で開発者3人までであり、それ以上は確実にレビューでタスクが詰まる

    • そもそもプロマネが病気とかトラックに轢かれたらどうするのか

    • 妥当性が保証された仕様があれば、仕様の策定者でなくても実装が仕様に沿っているかどうかをレビューできる

  • レビューのチェック内容が事前に共有されているべき

    • NG/OK が事前に共有されていないからNGが出るので、最初から共有されていれば問題は起きない

    • 基本設計や、アーキテクチャと呼ばれるもので、事前に守るべきルールを決める。ルールはチーム内で適宜アップデートし洗練させていけば良い(つまりそういう体制作りが求められる)

  • コードレビューでレビュワーが実際に動作確認するのはできるだけ避けるべき

    • 動作確認は本来試験でやる事

    • 動作環境をレビュワーが用意するのはコストが大きい

      • ただしブランチごとに動作環境が自動的にデプロイされるなど、環境が整っている場合は別

    • 「コードがちょっと怪しいが、動かしてみないとよく分からないぞ」という状況はまず設計を共有できていない可能性が高い(あるいはテストコードが不十分)。

この辺を突き詰めていくと、コードレビューの効率を上げるにはコードレビュー以外のことに目を向ける必要がある、というのは説得力があるのではないかと思う。

じゃあ要件定義とか見積もりのレビューはどうやるんだよという話

残念ですが、これについてはレビューの前にその対象物の書き方からまとめる必要があるのと、 答えらしい答えが自分の中でまだ出てないという事で、また今度という事で。。

(この辺システマチックにやってる方がいたら教えていただきたい。。)

大枠として言えることは

  • まず、そもそもレビューの対象となる諸々の資料を文章に書いて残す

  • とにかく論理的な根拠が何かを確認していく

  • 実装前では不確実性なことが必ずあるので、何が不確実かを見分けられるようにして、その不確実性に潜むリスクが何かを考えられるようにする。

ではないかと思います。

レビューの推奨事項のおまけ

以下は内容が具体的なノウハウによりすぎているのでおまけとして紹介するもの。この辺は別の記事としてまとめ直すかもしれない。

  • レビュイーはレビューするための資料やPRの説明を記載するべき

    • 資料不十分ならその時点でレビューしないで却下で良い

  • 説明を見ても分からないときは対面(オンラインでも良い)で説明してもらった方が良い

    • コミュニケーション作業でもあるので、テキストだけにとらわれすぎない

  • レビューするときに、ロールを意識すると指摘を出しやすい

    • 保守する人(あるいは障害対応する人)としての観点

    • 機能拡張する人としての観点

    • リリース作業を行う人としての観点

    • システムに攻撃する人としての観点

    など

Footnotes

1

IEEE 1028-2008 は読んでは見たものの、具体的にレビューでどういう指摘をすれば良いのか正直よく分からなかった..

2

伽藍とバザールについては、こちらで日本語訳が読めます 伽藍とバザール

Software Development Process レビュー コードレビュー

Webシステム開発でのコードレビューより前のレビュー — ykrods note
https://www.ykrods.net/posts/2020/04/25/review-on-web-system-development/

Comments