レビューを分析してみる2020

この記事は GMOペパボエンジニア Advent Calendar 2020 の 22 日目の記事です！

さて最近は CRE(Customer Reliability Engineer)として活動しているのですが、その件については会社のテックブログで執筆している最中なのでそちらで、今年何書こうかなとネタを探していた時。

2020年、1月末から在宅勤務をしており当初はうまく仕事進められるかなぁと不安に思っていたのも束の間、リモートワークのスタイルは結構自分に合っていたのかなんなのか、CREを始める転機にもなったし自分の成長を少しずつ感じた1年だったので、ふと思い返していて新しいことを書くよりこれに思い当たりました。
「このサービスにジョインして結構コード書いてきたけど、お前は今まで受けたレビューを活かせているのか？」と・・・
明らかにこういう指摘減ってるじゃんとかそういう成長できてるかと・・・
自分のやった特定の施策、特定の仕事に関して振り返る機会は多いと思うのですがそういう振り返りって実はあんまりちゃんとしてなくない？

ということで今携わっているサービス minne にジョインして約2年半、自分の立てた PR に対してのレビュー内容を振り返ってみる。

自分の立てた PR の一覧を抽出してみる
PR のレビューコメント一覧を抽出
レビューコメント一覧が取れたぞ
もらったレビューを分類してみよう
分類後、可視化してみる
総括的な
- 分析内容について
- レビューを見返してみて

自分の立てた PR の一覧を抽出してみる

分析する対象は普段自分が一番 PR を出している web アプリケーションのリポジトリ(FWはRails）にする。
スプレッドシートに出力して確認しながら進めたいので Google Apps Script でがりがりやる。

GitHub の REST API はこれを使います。

const ACCESS_TOKEN = PropertiesService.getScriptProperties().getProperty('access_token');
// per_page は MAX: 100 が指定可能
const PULLS_URL = 'https://api.github.com/repos/{owner}/{repo}/pulls?state=closed&sort=updated&direction=desc&per_page=100&page='
const PR_SHEET = SpreadsheetApp.openById('xxx');

const OPTIONS = {
  'method': 'GET',
  'muteHttpExceptions': true,
  'Content-Type': 'application/json',
  'headers': {
    'Authorization': ' token '+ ACCESS_TOKEN,
  }
};

function getMyPullRequests() {  
  var my_pulls = [];
  for (var i=1; i<=3; i++) { 
    var info = UrlFetchApp.fetch(PULLS_URL + i, OPTIONS).getContentText();
    var pull_requests = JSON.parse(info);
    
    pull_requests.forEach(
      function(pr) {
        if (pr.user.login == 'hikari') {
          PR_SHEET.appendRow([pr.number, pr.title, pr.html_url, pr.created_at, pr.user.login]);
        }
      }
    )
  }
}

GET List pull requests の API では PR の作成者を指定してリスト取得することはできないので、APIを叩いて取得した PR の作成者が自分だったら google スプレッドシートに一行ずつ append するということをやっている。
そしてデフォルトで30件までしか取得しないので per_page と page で取得位置を設定しつつリクエストする必要がある。無闇に回数叩きたくない&これまでのPRの作成数は GitHub の画面上でも自分で把握できるのでそれを見つつ page の値をぽちぽち変えて UrlFetchApp.fetch() を叩いてたけど、PRの総件数の目星がある程度ついていたら上記のようにループ処理で取得しても楽そう。

f:id:ave-h:20201221010731p:plain — こんな感じでPR一覧がシートに出力される

number はレスポンスに含まれる PR の id （リファレンス的には pull_number ）

PR のレビューコメント一覧を抽出

PR_SHEET に自分が立てた PR のリストを出力したので、今度はそれを元にレビューコメントを取得。
「PR についてるコメント」と言っても色々叩いてみると、厳密には API で取得できる内容には種類がある

repos/{owner}/{repo}/issues/{pull_number}/comments
- List issue comments ただのPRへのコメントはこれが該当する
- 例として rails のこのPRだとこれとか
- issue&PR含めて id 振られてるので /issues/{pull_number} で PR の普通のコメントも取得できるというのは確かに納得
repos/{owner}/{repo}/pulls/{pull_number}/comments
- Lists all review comments for a pull request PRに紐づいた全てのレビューコメント　file diff に対してコメントされているものが該当していそう
- 例1, 例2
repos/{owner}/{repo}/pulls/{pull_number}/reviews
- approved ✅ とか request changes のコメントはこれに該当する　レビューした結果のコメント ということで確かに...

実際の使っているリポジトリでの運用的に、{repo}/issues/{pull_number}/comments も {repo}/pulls/{pull_number}/comments も「レビューコメント」として使われていることには変わりないのだが今回は {repo}/pulls/{pull_number}/comments で取得できる内容のみに絞る。なので {repo}/issues/{pull_number}/comments だと「そもそもこの PR でやろうとしてる施策ってさ〜〜〜」という内容がよく取れたりもして、それが重要なレビューであることももちろんあるけれど今回は省く。
（自サービスでは PR が立った時 bot が自動でコメントしてくれるセキュリティチェックリストとかはこれに該当するけど、今回大量にあるそれは省いて精査できて助かった）こういうレビューコメントの導線の使い方(?)も、ちゃんと精査しようと思うと正しい使い方を研究するの大事だなぁと思った。

今度は、上で出力した PR 一覧のシートを元に{repo}/pulls/{pull_number}/comments を叩いていき、取得したコメント内容を見やすいよう PR のタイトルやURLとともに別のスプレッドシートに書き込んでいく。

const COMMENT_SHEET = SpreadsheetApp.openById('xxx');

function writeCommentSheet() {
  raw = PR_SHEET.getLastRow();
  sheet = PR_SHEET.getActiveSheet();
  last_raw = sheet.getLastRow();
  
  for (var i=2; i<=last_raw; i++) { 
    // 初回はシートの2行目から1列目に書いてあるセルを取得する
    pr_number = sheet.getRange(i, 1).getValue();
    // 開始位置が2行・2列目のセルを取得
    pr_title = sheet.getRange(i, 2).getValue();
    // 開始位置が2行・3列目のセルを取得
    pr_url = sheet.getRange(i, 3).getValue();
    
    list = UrlFetchApp.fetch('https://api.github.com/repos/{owner}/{repo}/pulls/' + pr_number + '/comments', OPTIONS).getContentText();
    var comments = JSON.parse(list);

    if (comments.length) {
      comments.forEach(
        function(comment) {
          COMMENT_SHEET.appendRow([pr_title, pr_url, comment.body, comment.user.login, comment.created_at]);
        }
      )
    }
  }
}

getRange() の解説はこちらを参考。

レビューコメント一覧が取れたぞ

f:id:ave-h:20201221020841p:plain — ボカシだらけだけどこんな感じに出力される

まずここで、自分のこのリポジトリに出した（merge 済みの） PR 数は 267
総レビューコメント数: 1157 うち自分のコメントじゃないものは 693 というのが雑にフィルタするとわかる

f:id:ave-h:20201221021557p:plain — PR数 & コメント数の推移

年別で見ると 1 PR あたりの平均コメント数が

2018: 平均 3.7
2019: 平均 2.9
2020: 平均 1.6

になる。これについてはあとでも触れるけど 2018 年に比べると今年度すごい少ない。

※注
- 2019 年度は一つの PR につき1人が最初から最後まで完遂するのではなくチームでペアプロしながら進める運用方針であったため、PRは自分が立てているが他の人が半分くらい実装している（commit を積んでいる）ものもある
- が自分が関わっていた（何かしらコードを commit してる）PR には変わりないので自分以外の人の実装に言及しているレビューがあっても、それも含めてカウントする
- 逆に自分以外の人が立てたPRに自分が沢山 commit 積んでいるものもあるが集計が大変なので今回は省く

もらったレビューを分類してみよう

シートに吐き出された実に 693 のレビューコメントをルールを決めて分類していく。

もらったレビューってどういう指摘分類に該当するのか・・・？これがむずい

リーダブルコードを読み返して書き方一つとっても精緻に分類していくかと思ったけど、見始めると分類項が無限に出てくる

多すぎても振り返りしにくいので、シートの中を見てある程度絞り出せてきた段階で以下のようにまとめる。
※なお対象コメントは「レビューを反映して修正まで行った」ものに限るのではなく、指摘がありつつやり取りの中で直すべきものではないという結論になったものも分類している

f:id:ave-h:20201221024057p:plain — 自身で作成した指摘分類

我流の分類の中での補足
- 「システムエラーまたはバグを発生させるコードがある」
  - 仕様に沿った実装をしているがその中でバグを発生させるコードが生まれているというものを分類
- 「パフォーマンス改善の余地がある」
  - 発行されるクエリの最適化、テストの高速化等
- 「Railsの書き方」
  - Rails にある便利な AR のメソッドこれが使えるよとか MVC の責務についての話など
- 「DRY原則」
  - 端的に書いているけど https://qiita.com/yatmsu/items/b4a84c4ae78fd67a364c これに則っています
- 「可読性の向上」
  - 書き方の統一、使ってない変数や不要な処理がある、「関数名と処理内容の乖離」に該当しない命名の改善などもこれに含む

またどうしてもやり取りの中で生まれている話とか「なるほどわかった」的コメントなど分類できないものもあるので、全てのコメントをこの項目のどれかに絶対に振り分ける、というのはせず、どれにも該当しなかったら分類しない。

（何時間かかけて一気にやろう！とすると段々ダルくなってきてあれもAの項目、うんこれもAの項目...と雑になりそうなのが目に見えていたので一気消化はしていない。）

分類後、可視化してみる

ちまちまとシートのコメントを分類したら年別に分けて、その年の各項目での指摘のされ具合はどんなもんなのかグラフにしてみよう。

年別のレビューコメント数と指摘分類別の内訳

2018	2019	2020
256件	277件	160件

年別の指摘分類TOP3

	2018	2019	2020
1位	可読性の向上	パフォーマンス改善の余地がある可読性の向上（同率1位）	可読性の向上
2位	仕様に沿った実装になっていない	システムエラーまたはバグを発生させるコードがある	Railsの書き方
3位	Railsの書き方	仕様に沿った実装になっていない Railsの書き方（同率3位）	パフォーマンス改善の余地があるシステムエラーまたはバグを発生させるコードがある（同率3位）

2018年（ジョインして初めての年）は頭の中に仕様を落とし込んで仕様通り実装することに躓いていたっぽい。2020年にもあるにはあるけどかなり減っている
テストコードを書くのが苦手＆以前は結構つっこまれた記憶があったので テストケース不足 テストコードが実装が意図した通り動くかを確かめる内容になっているか あたりが 2018, 2019 は上位に来るかと思っていたが意外とそうでもない
- 昨年の記事のようにテスト書くのは結構得意になってきたからか 2020年に テストケース不足 の指摘は0件になった
- テストコードが実装が意図した通り動くかを確かめる内容になっているか の指摘は依然あるけど 😇 それでも減ってる
2019年はRails (&Ruby)を書くことに慣れてきたっぽいががむしゃらに書くのだけは慣れたのか、パフォーマンス面への視点はまだ足りておらず N+1 問題とかを発生させまくって指摘を受けていた感じがある

年別の特徴がなんとなく分かってきたが、言うほど特定の項目が劇的に減ってる&増えているというのが目に見えないしこの円グラフは内訳が多すぎて分かりにくいぞ...と思うのでさらにこれらの指摘を大きく分けて見てみることにする。

レビューの指摘分類が明文化されてないかな〜と調べたときにこちらで見つけた

qiita.com

記載の IPA 独立行政法人情報処理推進機構 p8~9の資料が分かりやすかったのでそれにならって上記の指摘分類項目をさらに分類してみる。

ソフトウェア品質とは以下6つの品質特性に分類されており、それぞれいくつかの副特性の定義とで構成されているというもの。

- 機能性
  - 目的から求められる必要な機能の実装の度合い 
- 移植性
  - 別環境へ移した際、そのまま動作する度合い
- 保守性
  - 保守（改定）作業に必要とする努力の度合い
- 効率性
  - 目的のために使用する資源の度合い 
- 使用性
  - わかりやすさ、使いやすさの度合い
- 信頼性
  - 機能が正常動作し続ける度合い

f:id:ave-h:20201222013032p:plain — 自身で作成した指摘分類をソフトウェア品質にならい分類してみる

ソフトウェア使用者、つまりサービスのエンドユーザの視点での理解性の面で大きな定義では「使用性」に分類できたレビュー指摘もあったな...😇（上の円グラフの集計上では分類できなかったもの）気付くのが遅すぎたな... となったけどこれはこれで完遂させる。

ソフトウェア品質特性で分類したレビュー指摘の分類

2018	2019	2020

さっきの円グラフよりだいぶすっきりしたものになった。（項目ごとにちゃんと色を統一もした....😇）

元の自身で作成した指摘分類が、保守性の面での指摘に分類されるものが多い & 保守性に分類した 可読性の向上 Railsの書き方 などは指摘数が一貫してある程度多いので想定通りではあったけど、

2019年はRails (&Ruby)を書くことに慣れてきたっぽいががむしゃらに書くのだけは慣れたのか、パフォーマンス面への視点はまだ足りておらず N+1 問題とかを発生させまくって指摘を受けていた感じがある

パフォーマンス改善の余地がある 等が分類される信頼性が2019年以降は保守性の次に多い。

f:id:ave-h:20201222022604p:plain

2020年現在、仕様に沿った実装になっていない テストコードが実装が意図した通り~ の機能性の面での指摘が最初の頃より減っているのはいいけど信頼性の面の考慮漏れが目立ってきてるので今後はいっそう見落とししないように注意 👀

総括的な

分析内容について

出すPRの数が増えて、それに対してレビューでの指摘が徐々に減っていっているという結果はよかった
指摘を分類してみると意外に劇的な変化は見られなかったけど、自分の視点が変わっていっているのがわかるので年単位でこうやって分析してみることで結構新たな発見がある
改めて今の自分が身についてない視点も炙り出せる
レビューの指摘分類・どこを重視すべきかを明文化したりチェックリスト化したりするのは難しい
- もちろんやってる内容によるのだけど、新規のプロジェクトとかでちゃんと開発チームで話し合ったりしておくと新しく入ってきた人にも明示できるしよいよなぁ
PR数とレビューコメント数を抽出して 1 PR あたりの平均コメント数が2020年は平均 1.6という結果ではあったが、きちんと見られた結果1.6 だったであればよいけどレビュワーの目の数が減っていっている結果がその数字だとあんまりよろしくない（多くの視点があればあるほど洗練されていくし不具合を未然に防げる可能性も上がる）
- 現状、チームの人数に対して見るPRの数が多くなっており、全員が全てのPRをレビューしなければいけないという運用にしていないため仕方ない側面もある
- ではレビュワーの目の数が減ってるし、それによってバグの数も増えていたりするのでは？となると、そのような実感はあまりないので単純に個々のスキルも上がる&視点も洗煉されていき、少数のレビュワーでもちゃんとレビューができておりバグを発生させている実績は少ない といえそう
- 実際年にどのくらいレビューで見落としたことでの不具合が発覚したか ってちゃんと計測してない気がするのでそこの振り返りは大事（自戒）
可視化ってむずくない？（自戒）（大事）
出した PR 他の人に比べてまだまだ少ないなぁと思うので来年は一年で一番 PR 出す人になる😎

レビューを見返してみて

各レビュワーのコメントをこの総量で眺めてみるとこの人はよくここを気にするんだなとか、説明の仕方に個性が出ていておもしろい
これすごいいいレビューだなぁとか、この自分のコメントはどういう判断した結果こうしてるとかわかりやすく言ってていいコメントだなぁとレビュワー（チームメンバー）へはもちろん自分への信頼度も高まる🤗
今回それぞれの指摘を分類してみたということをやってるけど レビューは自由でいいということ
- これは全然もっともな指摘ではないしコメントするのやめておこう... とためらうのはもったいない
- ちょっとした疑問でも知識が増えて返ってくるので上で レビュワーの目~ の話してるけど来年はもっとレビューする😡🔥🔥

総レビューコメント数: 1157 うち自分のコメントじゃないものは 693 というのが雑にフィルタするとわかる

レビューコメント 693 のうち、特にコメントをしてくれたのはチームメンバの @ogidow さん（149！！！？？？！）で次に @ebihara99999 さん（135！！！！！）だった　いつもありがとうございます！！！！！

そんなこんなで怒涛の一年ではあったけど 2021 年もよい年にするぞ！

そういえば今年は minne #買ってよかったもの Advent Calendar 2020 も書いたのでこちらもぜひ読んでください。

note.com

補習ほぼ確

学びや好きなことをただ自由に書く