どうしてレビューをする必要があるのか

2018-12-31

ウエイト重めな仕様変更のプルリク、簡単なテストコードの修正のプルリク、アプリケーションとは関係のない自動化ツールの作成のプルリク、、

と、いろいろあると思いますがそんなかなり身近なプルリクのレビューについて今回は記事にしたいと思います。

ちなみに私が所属しているチームの中でのプルリクの扱い方の書きますので参考程度にお読みください！！

人間はミスをする生き物です。

そのためレビューイ(レビューされる人)の判断でソースに問題がないと思っていても他の人がソースを見ると問題があるコードというのはかなり多く存在しています。

そのため、レビューという行為は必ず必要となるのです。

他にも、新人のプログラマは先輩のソースコードを見ることでソースの書き方や処理ロジックを参考にすることができるのです。 ⇒これもかなり大事なこと

できる先輩方のソースを見ると、「そう書くと簡潔に書けるのか」と感心することが多々あります。

そのため、プルリクのレビューはミスを発見する以外にも重要な工程なのです。

プルリクってなにさ

ここで簡単にプルリクについて書いておきたいと思います。

プルリクについてわかっている人はスキップしてかまいません。

SIの方でSVNしか触ったことがない人とかは必見です(私もSI系にいたときはSVNしか使用していないプロジェクトがあり最初は戸惑いました。。)

現在の多くのプロジェクトではGitを使用してソースコードを管理していると思います。

Gitを使用するとブランチという機能を使用して本幹のソースコードに影響を与えることなくソースコードを改修することが可能となります。

そのためGitを使用して開発や保守を行っているプロジェクトではブランチを切って作業を行うことが普通だと思います。

作成したブランチですが、本幹のブランチに統合(マージ)しないとデプロイとかのときに改修内容が反映されません。

その時にブランチをマージするときにプルリクエストという形で本幹のブランチにマージするリクエストを送ります。

これがプルリクのことです。

開発するときはGitのホスティングサービスのBitBucketやGitBucket、GitHubなんかを使用していると思いますがこの時ソースコードをマージするレビューアを決めソースコードのレビューをしてもらうイメージです。

プルリクを作成することの意義、レビューをすることの意義、プルリクについてを書いてきましたが、ここではプルリクの粒度について書きたいと思います。

大規模なソースコードの改修になると触るファイルや追加するコードの行数が多くなってきます。

この時1つのプルリクにたくさんのソースの改修内容を含ませてしまうと、本人は楽で良いですがレビューアからするとソースを読むだけでかなりの時間がとられてしまいます。

ですからレビューアのことも考えてプルリクの粒度をなるべく小さく、機能単位にしてあげることが重要です。

1つのプルリクでは1つの機能とそのテストくらいにとどめて複数のプルリクを作成してあげた方がレビューアからすると良い場合はたくさんあります。

そのためプルリクの粒度(大きさ)を意識してプルリクを送ることが重要となってくるのです。

プルリクをレビューするのはコーディングをするよりも時間がかかったりするものです。

複数人にレビューを依頼する場合、その人数と時間がレビューに費やされるためコーディングより時間がかかってしまうことが多くなるのです。

では、なぜレビューにそんな時間をかけるのか、ということですが、そのソースコードの改修を本幹のブランチに入れるということはそのまま本番反映に直結することも多いのです。

そのため、改修をした本人からは「簡単なソースコード改修だから早くマージしてくれ」と思ってもレビューアからするとそう簡単なことではないのです。

本番反映をしてからそのソースに問題があることが分かったとしてその分のエラー部分の修正をする方がよっぽど工数がかかってしまいます。 (調査とかから入り、類似箇所、影響範囲なども考えないといけないため)

そのため、責任者というのは本番反映に関して注意を払っているのです。

簡単にプルリクの粒度とレビューに関して書きましたが、ここで私の所属しているチームでのプルリクの取り扱いについて書きたいと思います。

私の所属しているチームではプルリクの承認をするだけでマージは本人が行うという方式をとっています。

これは改修したソースの責任の所在をはっきりさせるために行っています。

基本的にはブランチのマージはレビューアの人が行うと思いますが、あえてレビューイが承認を受けたうえでマージすることで責任をレビューイが負うということになっています。

最初は責任を負うから改修をしたくないと思うかもしれませんが、本人が責任を負う形をとることで当事者意識を持ち改修をに取り組むようになるため効果的とのことでこの形式をとっています。

是非、参考にしてみてください！

ちなみに私のチームは主にインフラ面を扱っているのですが自動化ツールやansibleなどの構成管理ツールの面でのプルリクが多く、がっつりソースをいじることが少ないためこの方式がより適しているのかもしれません。