2020年度新卒、くさころ (9sako6)です。Rails アプリケーションのプロジェクトにアサインされてもうじき2ヶ月になり、その間に15くらいの Pull Request (以降、PR)を出しました。PR についたコメントは合計400弱、そのうちのある大きめの PR の Conversation 数は200を超えました (bot を含む)。コメントのほとんどはコードレビューによるもので、先輩方の丁寧で熱心な指導の賜物です。

本記事では、コードレビューを通して私が学んだ知識のうち、 DX: Developer Experience (開発者体験)の向上に不可欠だと思うものをご紹介します。

コミットへの意識

入社した最初の頃に教わったのは「コミットメッセージには変更理由を書く」ということです。

5W1H のうち、"Why" 以外はコミットそのものを見れば明らかです。しかし、"Why" (なぜ)その変更が必要だったかは、コミットメッセージに含めない限り読み取れないことがあります。コードだけでは伝えられない意図を伝えるために、コミットメッセージには変更理由を書くとよいです。

以下、コミットメッセージの例です。1行目にコミットの説明、2行目に空行、3行目以降に変更理由を書いています。

    Fix a BarPerfTool's error when using foo

    BarPerfTool で foo を使うと RidiculousError が起こるため bar を使う。
    ちなみにこれはドキュメントにも書かれていない。

    ```
    RidiculousError: Use bar instead of foo!!!
    ```

koic.hatenablog.com

加えて、変更理由を意識するとコミットをどんな粒度にすべきか見えてくると思っています。例えば、リファクタリングと機能追加では変更理由が異なります。前者には「4つのファイルを1つにまとめるため」といった理由が、後者には「ユーザー一覧表の描画を高速化するため」といった理由があるはずです。

したがって、自ずと別コミットに分けるべき変更であることがわかります。機能追加中にリファクタリングしたい別の箇所が見つかったからと言って、1つのコミットに入れないようにします。

逆に、変更理由を意識した粒度のコミットは、git blame や git log で履歴を追う際（コードレビュー時、過去実装を参考に機能追加・バグ修正時 etc.）に混乱がなく、十分な情報をもたらすので DX に貢献します。

手元で作業中のコミットは自由にやってよいと思いますが、PR を出す際は他の人が読むコミットになることを考慮して rebase ないし squash で変更理由単位になるよう整理しておきます。

スローテストへの意識

CI 待ち時間が長くても良いことはないので、テストははやくしたいです。そのためにできるコードレベルでの貢献は、無駄な処理をしないことです。

例として、sleep を挙げます。JavaScript で非同期処理する画面のテストの際、描画が完了していない要素へのアクセスが発生することがあります。その場しのぎ的に sleep 1 といった決め打ちの待ちを挟めば解決できることはありますが、その分だけ確実に実行時間が増えます。こんな時、Capybara の matcher は要素が見つかるまでループしてくれます。¹ 非同期処理を待ちたい時、まずは matcher の使用を検討するのが良いと思います。

# 'やる気スイッチ' が見つかるまでループする。
# ループ時間上限は Capybara.default_max_wait_time
expect(page).to have_content('やる気スイッチ')

他に遅さを生み出すのは、E2E テストです。ブラックボックステストなので価値がありますが、ブラウザをエミュレートする分時間がかかります。そこで、E2E テストではなく、Model 等のテストで済ませられないかを考えます。私の場合だと、E2E テストにはゴールデンパスのテストといくつか不安な点のテストを書きます。そして Model のテストでコーナーケースをつついています。

その他の細かい例として、「不要になったテストは消す」「用意するサンプル数を最小限にする」「ページネーションの閾値を下げてテストする」等が挙げられます。