PRレビュー指摘を自動化しました (EOF + new line)

こんにちは、ヘルスケア開発部でエンジニアをやっている大澤です。

本日はPRのレビュー指摘の自動化として一枚で完結するgithub actionを作成しましたので、
汎用性がある程度高いのでこちらを紹介することにしました。

作成した動機

弊社の開発部では github flow (厳密には異なります) を利用して開発を行っております。
業務時に PR をやり取りしています。

しかしながらレガシープロジェクトで template engine用の linter が入っていません。
部内コーディング規約で決まっている "EOF の直前は必ず改行" を入れるというものが決まっておりまして、 linter が無いため、この修正を忘れてしまい、 "EOF の直前は必ず改行してください" という指摘を受けて差し戻しを受けてしまうことが何度か有りました。

添付画像のような状態のときに指摘を受けるわけですね。
- - 左: EOFだけ
  - 右: new line + EOF の場合
    - 🚫 の有無で判断が可能です。

コーディング規約の違反の指摘をしていただけるのはありがたいです。
しかし、本来 PR はロジックを見て問題がないことを確認する作業が大きいです。
コーディング規約違反ならば linter を入れて自動的チェック・自動修正を入れたりできれば、
レビュアーもレビュイーもより本質的なコードレビューや議論に時間を割くことができます。

コーディング規約の違反の指摘についてCI化し、自動化しようと思い立ちました。

課題

レガシープロジェクト故に単純にlint checkを入れると過去の数百、数千といったファイルについて差分が発生してしまう。
- linter導入時に大きなチェックが伴うとlinter の導入自体が難しくなりがち
プログラミング言語と異なる template engine の linter が .editorconfig にしか存在しない。
- eclint という.editorconfig をコーディング規約をチェック・修正するツールは有るが ignore 設定が存在しない

解法

レガシープロジェクト故に単純にlint checkを入れると過去の数百、数千といったファイルについて差分が発生してしまう。
template engine の linter が .editorconfig にしか存在しない。
- => PRで発生する差分ファイルのみを対象に linter の CI を実行するように変更
  - => できたのが上述のgithub action

また下記の個人的な要望を満たすものとして作成しました。

サードパーティ製のgithub actionではなく、なるべく公式やコミュニティーが出しているものを利用する
- サードパーティ製のgithub actionを避けることでセキュリティー的にも若干強固するためですね。
github action を新規作成するのは保守の面から面倒なのでなるべく避ける

github action の中身

下記の内容をリポジトリ直下の下記のようなファイルとして設置していただくだけで利用できます。

.github/workflows/check_eof_with_new_line.yml

name: CHECK-EOF-WITH-NEW-LINE

on:
  pull_request:
    types: [synchronize, opened, reopened]
    branches:
      - '**'

jobs:
  check-eof-with-new-line:
    runs-on: ubuntu-latest
    steps:
      # clone repository to be able to merge master
      - name: checkout repository
        uses: actions/checkout@v2
        with:
          ref: ${{ github.event.pull_request.head.ref }}
          fetch-depth: 0
      - name: setting for git
        # 1.2. setting for git merge command (require user.email and user.name)
        run: |
          git config --global user.email "xxxxxx.ci@users.noreply.github.com" && \
          git config --global user.name "xxxxxx.ci"
      # install eclint only
      - uses: actions/setup-node@v2
        with:
          node-version: '12'
      - run: |
          rm package.json package-lock.json || :
      - run: npm init -y
      - run: npm install -g eclint
      # get diff files and eclint check files
      - run: |
          rm .editorconfig || :
      - run: |
          for j in  $( \
                  git --no-pager diff --name-only ${{ github.event.pull_request.head.ref }}  origin/${{ github.event.pull_request.base.ref }} | xargs file | grep ".*: .* text" | sed "s;\(.*\): .* text.*;\1;" \
              ); \
          do
              eclint check "$j" --insert_final_newline; \
          done

github action について行ごとの詳細な説明

checkout

fetch-depth: 0 というオプションを利用することで branch の情報が残るようにして checkout する
- このオプションを付けないと current branch を squash して 1 commit だけになるように checkout してしまいます。
  - Q.つけないとどうなるの？
  - A.この後に行う git diff で branch 等を指定しても存在しないとエラーが発生してしまいます。

- name: checkout repository
  uses: actions/checkout@v2
  with:
    ref: ${{ github.event.pull_request.head.ref }}
    fetch-depth: 0

git の設定を追加

一部 git command では user の情報を要求してくるので、適当に宣言しておきます。

- name: setting for git
  # 1.2. setting for git merge command (require user.email and user.name)
  run: |
    git config --global user.email "xxxxxx.ci@users.noreply.github.com" && \
    git config --global user.name "xxxxxx.ci"

eclint コマンドを install します。

node、npm と eclint を install しておきます。
global install なので関係はありませんがなんとなく package.json や package-lock.json も消しておきます。
- ※ 元々 local install で実行しようとしたけど上手く行かなかったので残ってしまった部分(消し忘れとも言う)

# install eclint only
- uses: actions/setup-node@v2
  with:
    node-version: '12'
- run: |
    rm package.json package-lock.json || :
- run: npm init -y
- run: npm install -g eclint

余計な指摘をしないように.editorconfigを削除する。

.editorconfig を削除します。
- ちなみにコマンドの後ろについている || : は対象のファイルが無くてもコマンドのexitステータスを0にする小技
※ 今回は "EOF の直前は必ず改行" だけの指摘を行いたいので、他の指摘をしないためにすでに .editorconfig が存在していれば削除します。
- ※ もし "EOF の直前は必ず改行" だけではなく .editorconfig の規約違反を全て検知してほしい場合は削除は不要です。

- run: |
  rm .editorconfig || :

差分ファイルのみを対象に CI を実行する

git diff の機能を利用して PR の差分対象ファイル(binaryを除く)を抽出
- git --no-pager diff --name-only ${{ github.event.pull_request.head.ref }} origin/${{ github.event.pull_request.base.ref }}
  - PR 上で差分ファイルとして出てくるファイル一覧を取得します。
- xargs file | grep ".*: .* text" | sed "s;$.*$: .* text.*;\1;"
  - 差分ファイルとして出てくる一覧にはバイナリが入っている事があるのでこれを取り除きテキストファイルのみの差分ファイル一覧を取得します。
for で check を実行する
- 上記で取得したファイルについて eclint 逐次実行します。

- run: |
      for j in  $( \
      git --no-pager diff --name-only ${{ github.event.pull_request.head.ref }}  origin/${{ github.event.pull_request.base.ref }} | xargs file | grep ".*: .* text" | sed "s;\(.*\): .* text.*;\1;" \
          ); \
      do
          eclint check "$j" --insert_final_newline; \
      done

まとめ

この github action を設置することで以下の事柄について解決することができました。
- linter を導入時に CI 導入のために大きな修正差分が発生する
  - OR 導入時に常にredなCIになる
- "EOF の直前は必ず改行" という指摘の往復をなくして効率化
CI の実行例
- red
- green: