Dangerで始めるPull Requestチェック自動化

f:id:Utmrer:20170228201418p:plain

こんにちはー！こねひとちほーのえんじにあのフレンズ@Utmrerだよー！

今回はPull Requestを自動でチェックしてくれるDangerについて紹介します。

Pull Requestでのコミュニケーション

Pull Requestのレビューは不具合の指摘やコーディングスタイルの統一、より良いコードのための提案などのために行われます。ですが、次のようなコミュニケーションをしたことはありませんか…？

タイトルにIssue Idを含めてもらえますか？
WIPみたいなんですがレビューして大丈夫ですか？
Base branchが間違ってます、変更してください。
変更履歴のdocsを更新してください。

このような「実装とは関係のない指摘」はできるだけ減らし、自動化したいものです。それを実現するのがDangerです。

Dangerとは

DangerのGitHubには次のように書かれています。

Formalize your Pull Request etiquette.

Stop saying “you forgot to …” in code review

つまり「Pull Requestの作法を形式化して指摘を自動化」するツールです。

DangerがチェックするのはPull Requestの体裁なのでそのPull RequestがSwiftのプロジェクトであろうがRubyであろうが関係なく、すべてのプロジェクトに導入可能です。実際にRxSwiftというSwiftのプロジェクトやcapistrano、sentryなど様々なプロジェクトで導入されています。

Dangerの導入

Travis上でDangerを実行し、GitHubのPull Requestにコメントするところまでを説明します。

Gemfileの設定

GemfileにDangerを追加します。これだけでTravis上でDangerを実行できます。

source 'https://rubygems.org'

gem 'danger'

Dangerfileの追加

プロジェクトのルートディレクトリにDangerfileというファイルを追加します。

このファイルにDangerにどのような項目をチェックしてもらうかを記述していきます。

warn('あぶないよー！')

warn(message)と書くと「これは注意が必要だよ」とGitHubにコメントします。

通常は特定の条件によってコメントするのでifなどで囲みますが今回はこのままにしておきます。

.travis.ymlの設定

.travis.ymlにDangerを実行するように設定します。

cache:
  - bundler
before_script:
  - bundle exec danger

Cache機能を使えば毎回bundle installを実行する必要が無いので時間節約のためにおすすめです。

Travis環境変数の設定

TravisからGitHubへコメントするため、TravisにGitHubのアクセストークンを設定する必要があります。アクセストークンはこちらのページから作成できます。

Dangerでは公開プロジェクトではpublic_repo、非公開プロジェクトではrepoの権限を持ったアクセストークンが推奨されています。

また、この時にアクセストークンを発行するGitHubアカウントは普段使っている個人アカウントではなくBot用のMachineアカウントの方が良いです。GitHubは1人1アカウントのMachineアカウントの作成を許可しています。

発行したアクセストークンはTravisの各Repositoryのsettingsページで設定します。 f:id:Utmrer:20170228161943p:plain Environment VariablesにDANGER_GITHUB_API_TOKENというNameで先程のアクセストークンを追加します。

Pull Requestの作成

以上でDangerの最低限の設定は終了です。Pull Requestを作成するとDangerからコメントが来ます。 f:id:Utmrer:20170228200733p:plain

ここまでの導入方法はDangerの公式HPにも載っています。 GitLabを利用している方やCircle CIを利用している方はこちらからご自分の環境にあった方法で導入してください。

構文

DSL

Dangerにはコメントを書き込むDSLがいくつかあり、最もよく使うであろう3つのDSLを紹介します。

📖 message

messageは単純にコメントを書き込むDSLです。定型文を返す時などに使えそうですね。

message('Pull Requestありがとうございます。レビューまで少々お待ち下さい。')

⚠ warn

warnは出来れば修正して欲しい、もしかしたら違反しているかもという場合に使います。Pull Requestのタイトルやディスクリプションに関するものはwarnがいいでしょう。

warn('descriptionを300字以上書いて下さい')

🚫 fail

failは許容できない内容の場合に使いましょう。このDSLを利用した時、Commit stateはfailureになります。

fail('LICENSEファイルは変更しないで下さい。')

変数

環境によってDangerfile上で扱える変数がいくつかあります。これらを組み合わせることでルールを作っていきます。今回はGitHub環境で使えるgitとgithubについて紹介します。

git

gitという変数にはPull Requestの対象となっているCommitの情報などが含まれています。例えば「編集したファイルの一覧」「削除した行数」などです。

if git.deleted_files.include? "LICENSE"
  fail('LICENSEファイルは変更しないで下さい。')
end

github

githubという変数からPull Requestの情報が取得できます。github.pr_jsonからPull Request全体の情報を取得でき、GitHubのAPI Referenceなどを読むとgithub.pr_jsonにどのような値が入っているかわかるでしょう。

よく使うデータには専用の関数が用意されていてgithub.pr_titleやgithub.pr_bodyからタイトルやディスクリプションがをシンプルに取得できます。

if !github.pr_json['mergeable']
  fail('マージできません。')
end
if !github.pr_title.include? "WIP"
  warn('このPull Requestは実装途中です。')
end

詳しくはReferenceをご覧ください。

Danger導入事例

Dangerの導入方法がわかったところで、OSSプロジェクトがDangerでどのようなことを行っているか見てみましょう。

capistrano

capistranoではDangerfileで次のようなことをチェックしているようです。

has_lib_changes = !git.modified_files.grep(/^lib/).empty?
has_test_changes = !git.modified_files.grep(/^(features|spec)/).empty?
has_changelog_changes = git.modified_files.include?("CHANGELOG.md")
if has_lib_changes && !has_test_changes
  warn("There are code changes, but no corresponding tests. "\
       "Please include tests if this PR introduces any modifications in "\
       "#{project_name}'s behavior.",
       :sticky => false)
end

「Libraryに変更があったならテストを書こう」ということをチェックしていますね。Reviewerに指摘される前にTestが書ければコミュニケーションが減って時間の節約になります。

if !has_changelog_changes && has_lib_changes
  markdown <<-MARKDOWN
Here's an example of a CHANGELOG.md entry (place it immediately under the `* Your contribution here!` line):
 ```markdown
* [##{pr_number}](#{pr_url}): #{github.pr_title} - [@#{github.pr_author}](https://github.com/#{github.pr_author}).
 ```
MARKDOWN
  warn("Please update CHANGELOG.md with a description of your changes. "\
       "If this PR is not a user-facing change (e.g. just refactoring), "\
       "you can disregard this.", :sticky => false)
end

またLibraryに変更があったならCHANGELOG.mdに変更点を書くように促しています。これは商用プロダクトでもHelpの更新や仕様書の更新などに使えそうなロジックです。

capistrano/Dangerfile

Sentry

SentryではPull Requestの体裁に関するものがいくつかあります。

# Make it more obvious that a PR is a work in progress and shouldn"t be merged yet
warn("PR is classed as Work in Progress") if github.pr_title.include? "[WIP]" || github.pr_body.include?("#wip")

タイトルとディスクリプションをチェックしてPull RequestがWIPかどうかを判定しています。WIPなのにReviewをお願いすることを防げますし、Reviewerも「WIPなので見なくて大丈夫！」と判断できます。

@S_BIG_PR_LINES ||= 500
# Warn when there is a big PR
warn("Big PR -- consider splitting it up into multiple changesets") if git.lines_of_code > @S_BIG_PR_LINES

Pull Requestの実装行数が大きすぎる場合は分割するように促しています。実装内容が膨大なPull RequestはReviewが大変ですし、不具合が紛れ込む可能性も高いので分割してPull Requestを作るのは良いことですね。

sentry/Dangerfile

mamariQ

最後に、弊社のママリQ iOSアプリのDangerfileを公開します。

# --------------------
# test
# --------------------
is_source_changed = !git.modified_files.grep(/^mamariQ\/Classes/).empty?
is_test_changed = !git.modified_files.grep(/^mamariQTests\/Case/).empty?

if is_source_changed && !is_test_changed
  warn('コードの変更がありますが、テストの変更がありません。必要であればテストを追加・修正しましょう。')
end

# --------------------
# base branch
# --------------------
is_to_master = github.branch_for_base == 'master'
is_to_release = !!github.branch_for_base.match(/release-[0-9]+\.[0-9]+\.[0-9]/)
is_from_release = !!github.branch_for_head.match(/release-[0-9]+\.[0-9]+\.[0-9]/)

if is_to_master && !is_from_release
  warn('masterへmerge出来るのはrelease branchのみです。')
end

if is_to_release
  warn('release branchに対してPRを向けないで下さい。develop branchに向けてPRを作成し、develop branchをrelease branchにmergeしてください。')
end

# --------------------
# pr title
# --------------------
is_wip = github.pr_title.include? '[WIP]'

if is_wip
  warn('このPRは作業中です。')
end

is_to_develop = github.branch_for_base == 'develop'
is_start_with_issue_id = !!github.pr_title.match(/^#[0-9]+/)

if is_to_develop && !is_start_with_issue_id && !is_wip
  warn('PRのタイトルは<b>「#XXX Foo bar...」</b>とIssue idから始めてください。')
end

# --------------------
# diff size
# --------------------
is_big_pr = git.lines_of_code > 500

if is_big_pr
  warn('PRの変更量が多すぎます。可能であればPRを分割しましょう。分割が難しければ次から気をつけるようにしましょう。')
end

弊社のiOS開発ではgit-flowを採用していて、Pull RequestにおけるBase branchの選択がちょっと複雑です。そのため、DangerでBase branchのチェックを自動化しています。このDangerfileを書いたのは私なのに自分で引っかかることがあってDangerに感謝感激です。

人は人にしか出来ないことを

DangerにはPlugin機能があり、機能を拡張できるので可能性は∞です。Android LintやSwiftLintのPluginもあります。(私の環境ではSwiftLint Plugin動かなかったので動作は保証できないですが…)

そういえば、Dangerを作ったOrtaさんとKrauseFxさんはtry! Swiftで講演されるようですよ。try! Swiftに行かれる方はDangerの話もしてみてはどうでしょう？私もいきます☺

機械に出来ることは機械に任せて、人は人にしか出来ないことに集中していきましょう。

コネヒト開発者ブログ