コードレビューの機械的な指摘はDangerに任せる

先日のコードレビューの機械的な指摘はSideCIに任せるに続き、今回は同様のことが可能であるOSSのDangerについて紹介します。

Danger とは

Dangerとは、Pull Requestのレビュー時に発生しやすい、 ”You Forgot To...(...するのを忘れてませんか？）"という指摘を自動化するツールです。

danger.systems

事前に指摘する内容をコードで記述することで、CI上でPull Requestを解析して自動でコメントしてくれます。

課題

Pull Requestをレビューする中で、稀に以下の指摘が発生することがありました。

PRの本文に説明が無い、又は不足している
UIの変更があるのにスクリーンショットが貼られていない
変更するべきではないファイルが変更されている
1つのPRに対して変更が多い
（WIP中のPRに）WIPラベルが貼ってない

さらに弊社ではIssue管理をYouTrackで行っており、PRのタイトルとコミットメッセージの先頭には該当するIssue番号を付けるというルールがあります。

これらの指摘事項が発生した際にレビュアーが都度コメントするのは労力が掛かりますし、コードとは関係ない手戻り作業が発生してしまう可能性もあります。

そこでこれらのチェックを自動で機械的に即時に行うため、Dangerを導入しました。

効果

事前に記述したチェック項目に該当する場合は、下記のようなコメントが表示されます。

f:id:lcl-engineer:20171229211046p:plain

これはPRの作成、又はPUSHした際にCI上でDangerが実行され即時に反映されます。

そのため、PR作成時やPUSHした時点で誤りを把握できるので、レビュアーがチェックしてから修正が発生するという手戻りが減りました。また機会的に指摘されることで、小言を言う/言われる心理的負担も減りました。

それでは、導入手順をローカル環境とCI環境に分けて紹介します。

ローカル環境

導入方法

CI上でも同じ環境を作るためにBundlerでGemの管理をします。

Gemfileを生成

$ bundle init

Gemfileを編集

source "https://rubygems.org"

git_source(:github) {|repo_name| "https://github.com/#{repo_name}" }

gem "danger" // ← 追加

Dangerfileを生成

$ bundle install
$ bundle exec danger init

Dangerfileを更新

warn "変更箇所が200行を超えています。可能であれば分割しましょう。" if git.lines_of_code > 200

上記のコードで、変更箇所が200行を超えていた場合に警告を出すことができます。

次に動作確認を行います。

事前準備

Dangerを動作させるには、PRにアクセスするためのAPI トークンが必要です。

今回はGitHubを例に進めます。

まず、リポジトリにアクセス可能なアカウントのAPI Tokenを取得します。

https://github.com/settings/tokens

個人のアカウントで十分ですが、bot専用アカウントを作ると機械的に指摘されたコメントということが区別できてわかりやすいです。

Webhockの設定でPull Requsetにもチェックが入っているか確認

f:id:lcl-engineer:20171229211106p:plain

動作確認

以下のコマンドでチェックしたいPRを指定して実行します。これらのコマンドで実行した内容はPRへ反映されません。

$ DANGER_GITHUB_API_TOKEN={API_TOKEN} bundle exec danger pr {PR_URL}

既にマージ済のブランチに対してはdanger localコマンドを使うことで動作が確認できます。

$ DANGER_GITHUB_API_TOKEN={API_TOKEN} bundle exec danger local --use-merged-pr=[#id]

Dangerfileのコードを変更した際にキャッシュが効いて反映されない場合があります。その際は--clear-http-cache オプションでキャッシュをクリアすることが可能です。

CI環境

弊社ではモバイルアプリはBitriseを、その他のリポジトリではCircle CIを使用しています。

Bitriseの設定

API Tokenを登録

f:id:lcl-engineer:20171229211125p:plain

Workflowを追加

f:id:lcl-engineer:20171229211150p:plain

Triggersを追加

f:id:lcl-engineer:20171229211159p:plain

以上でBitriseの設定は完了です。指定したブランチに対してPRが作成された際にDangerが実行されます。

CircleCIの設定

GitHubと連携することで、Github側の設定は不要になります。

デフォルトでは全てのコミットでビルドが走ってしまうため、プロジェクトの設定でプルリクのみ走るように変更します。

f:id:lcl-engineer:20171229211601p:plain

プロジェクトのリポジトリにcircle.ymlを配置します。

下記のサンプルはCircle CI v1.0の書き方です。

machine:
  timezone:
    Asia/Tokyo
dependencies:
  cache_directories:
    - "vendor/bundle"
  override:
    - bundle -j4 --path=vendor/bundle
test:
  override:
    - bundle exec danger

実装例

弊社で利用している実装例を一部紹介します。

プロジェクト共通のチェック項目

# 対象コード以外についてのメッセージは除く
github.dismiss_out_of_range_messages

# ===== Title =====

# PRのタイトルにYouTrackのIssue番号が含まれているか（文字列-数字の塊）
match = github.pr_title.match /\w+-\d+/
if match
  # YouTrackへのリンクを貼る
  markdown "## YouTrack Ticket", "<a href='https://XXX.myjetbrains.com/youtrack/issue/#{match[0]}'>#{github.pr_title}</a>"
elsif !github.branch_for_head.match(/release\/\d.+/)
  warn "PRのタイトルはYouTrackのIssue番号から始めてください。"
end

if github.pr_title.split(' ').size < 2
  fail "PRのタイトルは、「Issue番号 Issueタイトル」形式にしてください。"
end

# ===== Description =====

# 本文が1行以上書かれているか（10行以下の軽微な変更は考慮）
warn "作業内容について本文に1行以上の説明を記載してください。" if github.pr_body.length < 1 && git.lines_of_code > 10


# ===== Commit message =====


# コミットメッセージにYouTrackのIssue番号が含まれているか
has_youtrack_issue_commit = git.commits.any? { |c| c.message =~ /\w+-\d+/ }
warn "YouTrackのIssue番号から始まっていないコミットメッセージがあります。" unless has_youtrack_issue_commit


# ===== Label =====

labels = github.pr_labels

# ラベルが付いていなかったら「wip」ラベルを付ける
add_label "wip", 'fbca04' if labels.empty?

Webのチェック項目

# ===== Code =====

warn "変更箇所が200行を超えています。可能であれば分割しましょう。" if git.lines_of_code > 200

if git.modified_files.include? "lib/XXX-module"
    warn 'sub moduleが変更されています。意図した更新か確認してください。'
end

 if git.modified_files.include? "app/views/pc/*"
    warn 'PC ERBが変更されています。タブレットに影響がないか確認してください。'
end

iOSのチェック項目

# SwiftLintのチェック
swiftlint.lint_files inline_mode: true
swiftlint.config_file = '.swiftlint.yml'

protected_files = ["Podfile", "Cartfile", "Dangerfile", "Gemfile", ".gitignore", ".swiftlint.yml"]
protected_files.each do |file|
  next if git.modified_files.grep(/#{file}/).empty?
  message("#{file}に変更が加えられています。")
end


if github.branch_for_head.start_with?("release")
# Deliverfileの変更されているか
warn "リリースノートを変更してください <a href='https://github.com/XXX/blob/develop/fastlane/Deliverfile'>Deliverfile</a>", sticky: true unless git.modified_files.include?("fastlane/Deliverfile")

  
view_extensions = [".xib", ".storyboard", "View.swift"]
has_view_changes = git.modified_files.any? { |file| view_extensions.any? { |ext| file.end_with? ext }}
pr_has_screenshot = github.pr_body =~ /https?:\/\/\S*\.(png|jpg|jpeg|gif){1}/
warn("見た目に変更がある場合は、スクリーンショットを添付してください。") if has_view_changes and !pr_has_screenshot

Swiftlintを使う場合は、Gemfileにdanger-swiftlintを追加します

source "https://rubygems.org"

git_source(:github) {|repo_name| "https://github.com/#{repo_name}" }

gem "danger"
gem "danger-swiftlint" // ← 追加

※ 一部、以下で紹介されているコードを参照してます

WEB+DB PRESS Vol.99：｜gihyo.jp … 技術評論社

（おまけ）別のリポジトリにあるDangerfileを参照

各リポジトリでDangerfileを用意してチェック項目を記述していますが、組織独自のルールなど共通する処理は専用リポジトリで管理して参照するようにしています。

例えば、danger-lclというリポジトリを用意して共通のチェック項目を記述したDangerfileを管理します。そして別のリポジトリのDangerfileで下記を追加することで、そのリポジトリのDangerfileの内容を読み込むことができます。

# danger-lclを読み込み
danger.import_dangerfile(github: "XXX/danger-lcl")

エンジニアを募集しています

LCLでは業務改善に意欲的なエンジニア積極的に募集中です。
興味のある方はお気軽にご連絡よろしくお願いいたします。
インターンも募集しています。

https://www.lclco.com/recruit/

LCL Engineers' Blog

バス比較なび・格安移動・バスとりっぷを運営する LCLの開発者ブログ