DangerΛ࢖ͬͯ PRΛࣗಈతʹνΣοΫ͢Δ 2017.08.19 Kyoto.ͳΜ͔ #3 / #kyotoasterisk Satoshi Hachiya / @jpmartha_jp

No content

Satoshi Hachiya • גࣜձࣾ ookami ʢ౦ژʣͷ iOS σϕϩούʔ • iOS ΞϓϦ Player! Λ୲౰

Satoshi Hachiya • גࣜձࣾ ookami ʢ౦ژʣͷ iOS σϕϩούʔ • iOS ΞϓϦ Player! Λ୲౰ • try! Swift Tokyoɺtry! Swift NYC ͷΦʔΨφΠβʔ • try! Swift India ͷεϐʔΧʔ

Satoshi Hachiya • Founder of Pancake Meetup (Tokyo, San Jose...)

Agenda • Danger ͱ͸ • Danger Λಋೖ͢Δ • λʔϛφϧ্Ͱ Danger Λ࢖͏ • CI ্Ͱ Danger Λ࢖͏ • ׆༻ࣄྫ

No content

Danger ͱ͸ • Danger (Ruby) • http:/ /danger.systems/ruby/ • Danger JS • http:/ /danger.systems/js/

Danger (Ruby)

࣮ߦՄೳͳ؀ڥ Circle, Travis, Jenkins, Buildkite, BuddyBuild, Semaphore, TeamCity, Xcode Bots, Drone, Surf and Bitrise ίϝϯτՄೳͳ؀ڥ GitHub, GitLab and Bitbucket Diffs ͷऔಘ Git

Danger Λಋೖ͢Δ

Gemfile Λ࡞੒͢Δ • $ gem install bundler ίϚϯυΛ࣮ߦ • $ bundle init ίϚϯυΛ࣮ߦ

Dangerfile Λ࡞੒͢Δ • Gemfile ʹ gem 'danger' Λ௥Ճ • $ bundle install Λ࣮ߦ • $ bundle exec danger init Λ࣮ߦ

σϑΥϧτͷ Dangerfile # Sometimes it's a README fix, or something like that - which isn't relevant for # including in a project's CHANGELOG for example declared_trivial = github.pr_title.include? "#trivial" # 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]" # Warn when there is a big PR warn("Big PR") if git.lines_of_code > 500 # Don't let testing shortcuts get into master by accident fail("fdescribe left in tests") if `grep -r fdescribe specs/ `.length > 1 fail("fit left in tests") if `grep -r fit specs/ `.length > 1

λʔϛφϧ্Ͱ Danger Λ࢖͏

Dangerfile # Test warn("ͱ͜ΖͰ΋͏ҿΜͰΔ΍Ζ!ʁ @#{github.pr_author}")

ίϚϯυ࣮ߦʢOSS ͷ৔߹ʣ $ bundle exec danger pr https://github.com/jpmartha/LicensePlist/pull/1 Result Warnings: - [ ] ͱ͜ΖͰ΋͏ҿΜͰΔ΍Ζ!ʁ @jpmartha

CI ্Ͱ Danger Λ࢖͏

GitHubʢOSS ͷ৔߹ʣ • Danger ༻ͷ ΞΧ΢ϯτ࡞੒ • Personal access tokens Λઃఆ

Travis CIʢ·ͨ͸ CircleCI ͳͲʣ • ؀ڥม਺ DANGER_GITHUB_API_TOKEN Λઃఆ

Travis CI • .travis.yml Λ࡞੒ ... include: - os: osx language: swift osx_image: xcode8.3 script: - bundle exec danger ...

bundle exec danger Λ ࣮ߦ͢ΔʢOSS ͷ৔߹ʣ # Test ͱ͜ΖͰ΋͏ҿΜͰΔ΍Ζ!ʁ @#{github.pr_author}")

׆༻ࣄྫ

Danger 1/4 # Sometimes its a README fix, or something like that - which isn't relevant for # including in a CHANGELOG for example has_app_changes = !git.modified_files.grep(/lib/).empty? has_test_changes = !git.modified_files.grep(/spec/).empty? is_version_bump = git.modified_files.sort == ["CHANGELOG.md", "lib/danger/version.rb"].sort if has_app_changes && !has_test_changes && !is_version_bump warn("Tests were not updated", sticky: false) end # Thanks other people! message(":tada:") if is_version_bump && github.pr_author != "orta"

Danger 2/4 # Make a note about contributors not in the organization unless github.api.organization_member?("danger", github.pr_author) message "@#{github.pr_author} is not a contributor yet, would you like to join the Danger org?" # Pay extra attention if they modify the gemspec if git.modified_files.include?("*.gemspec") warn "External contributor has edited the Gemspec" end end # Mainly to encourage writing up some reasoning about the PR, rather than # just leaving a title if github.pr_body.length < 5 fail "Please provide a summary in the Pull Request description" end

Danger 3/4 # Let people say that this isn't worth a CHANGELOG entry in the PR if they choose declared_trivial = (github.pr_title + github.pr_body).include?("#trivial") || !has_app_changes if !git.modified_files.include?("CHANGELOG.md") && !declared_trivial fail("Please include a CHANGELOG entry. \nYou can find it at [CHANGELOG.md](https://github.com/danger/danger/blob/master/CHANGELOG.md).", sticky: false) end # Docs are critical, so let's re-run the docs part of the specs and show any issues: core_plugins_docs = `bundle exec danger plugins lint lib/danger/danger_core/plugins/*.rb --warnings-as-errors` # If it failed, fail the build, and include markdown with the output error. unless $?.success? # We want to strip ANSI colors for our markdown, and make paths relative colourless_error = core_plugins_docs.gsub(/\e\[(\d+)(;\d+)*m/, "") markdown("### Core Docs Errors \n\n#{colourless_error}") fail("Failing due to documentation issues, see below.", sticky: false) end

Danger 4/4 # Oddly enough, it's quite possible to do some testing of Danger, inside Danger # So, you can ignore these, if you're looking at the Dangerfile to get ideas. # # If these are all empty something has gone wrong, better to raise it in a comment if git.modified_files.empty? && git.added_files.empty? && git.deleted_files.empty? fail "This PR has no changes at all, this is likely an issue during development." end # This comes from `./danger_plugins/protect_files.rb` which is automatically parsed by Danger files.protect_files(path: "danger.gemspec", message: ".gemspec modified", fail_build: false) # Ensure that our core plugins all have 100% documentation core_plugins = Dir.glob("lib/danger/danger_core/plugins/*.rb") core_lint_output = `bundle exec yard stats #{core_plugins.join " "} --list-undoc --tag tags` if !core_lint_output.include?("100.00%") fail "The core plugins are not at 100% doc'd - see below:", sticky: false elsif core_lint_output.include? "warning" warn "The core plugins are have yard warnings - see below", sticky: false markdown "```\n#{core_lint_output}```" end junit.parse "junit-results.xml" junit.headers = %i(file name) junit.report

fastlane 1/2 # We generally try to avoid big PRs warn("Big PR") if git.lines_of_code > 500 # Show a warning for PRs that are Work In Progress if (github.pr_body + github.pr_title).include?("WIP") warn("Pull Request is Work in Progress") end # Contributors should always provide a changelog when submitting a PR if github.pr_body.length < 5 warn("Please provide a changelog summary in the Pull Request description @#{github.pr_author}") end if git.modified_files.include?("snapshot/lib/assets/SnapshotHelper.swift") warn("You modified `SnapshotHelper.swift`, make sure to update the version number at the bottom of the file to notify users about the new helper file.") end

fastlane 2/2 # To avoid "PR & Runs" for which tests don't pass, we want to make spec errors more visible # The code below will run on Circle, parses the results in JSON and posts them to the PR as comment containing_dir = ENV["CIRCLE_TEST_REPORTS"] || "." # for local testing file_path = File.join(containing_dir, "rspec", "fastlane-junit-results.xml") if File.exist?(file_path) junit.parse(file_path) junit.headers = [:name, :file] junit.report else puts "Couldn't find any test artifacts in path #{file_path}" end

RxSwift 1/2 # Warn about develop branch warn("Please target PRs to `develop` branch") if github.branch_for_base != "develop" && github.branch_for_base != "swift-3.0" # Sometimes it's a README fix, or something like that - which isn't relevant for # including in a project's CHANGELOG for example declared_trivial = github.pr_title.include? "#trivial" # 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]" # Warn no CHANGELOG warn("No CHANGELOG changes made") if git.lines_of_code > 50 && !git.modified_files.include?("CHANGELOG.md") && !declared_trivial # Warn pod spec changes warn("RxCocoa.podspec changed") if git.modified_files.include?("RxCocoa.podspec") warn("RxSwift.podspec changed") if git.modified_files.include?("RxSwift.podspec") warn("RxTests.podspec changed") if git.modified_files.include?("RxTests.podspec") warn("RxBlocking.podspec changed") if git.modified_files.include?("RxBlocking.podspec")

RxSwift 2/2 # Warn summary on pull request if github.pr_body.length < 5 warn "Please provide a summary in the Pull Request description" end # If these are all empty something has gone wrong, better to raise it in a comment if git.modified_files.empty? && git.added_files.empty? && git.deleted_files.empty? fail "This PR has no changes at all, this is likely a developer issue." end # Warn when there is a big PR warn("Big PR") if git.lines_of_code > 500

ΫοΫύουʢAndroidʣ ... # for PR # #### if github.pr_title.include? "[WIP]" || github.pr_labels.include?("WIP") warn("PR is classed as Work in Progress") end # Warn when there is a big PR warn("a large PR") if git.lines_of_code > 300 # Warn when PR has no milestone warn("A pull request must have a milestone set") if github.pr_json["milestone"].nil? # Warn when PR has no assignees warn("A pull request must have some assignees") if github.pr_json["assignee"].nil? ...

AndroidDangerSampleʢϝϧΧϦʣ has_milestone = github.pr_json["milestone"] != nil warn "͋ΕΕʁϚΠϧετʔϯ͕ઃఆ͞Εͯͳ͍Αʁ" unless has_milestone warn "WIPͳͷʁ͸΍͘ऴΘΔͱ͍͍Ͷʔʂ" if github.pr_title.include? "[WIP]" or github.pr_labels.include?("WIP") ...

GitHub ͔ΒऔಘͰ͖Δ಺༰ͷҰྫ • github.pr_author • github.pr_title • github.pr_body • github.pr_labels • github.pr_json["milestone"] • github.pr_json["assignee"] • etc.

iOS ༻ʹߟ͑ͯΈͨ

Dangerfile αϯϓϧ for iOS (1/2) # ϓϧϦΫΤετͷλΠτϧʹ `WIP` ؚ͕·Ε͍ͯͨΒܯࠂ warn("`WIP` ΍Μʁ") if github.pr_title.include? "WIP" # ϓϧϦΫΤετʹ `Description` ؚ͕·Ε͍ͯͳ͔ͬͨΒܯࠂ warn("Description ͳ͍Ͱʂ") if !github.pr_body.include? "Description" # Ϛʔδઌ͕ `master` Ͱͳ͚Ε͹ܯࠂ message("`master` ޲͍ͯͳ͍Ͱʂ") if !github.branch_for_base == "master"

Dangerfile αϯϓϧ for iOS (2/2) # ௥Ճɾ࡟আͨ͠ߦ਺͕ `500` ߦΛ௒͍͑ͯͨΒܯࠂ warn("`500`ߦ௒͑ͱΔΘʂ") if git.lines_of_code > 500 # `App.xcodeproj/project.pbxproj` Λมߋ͍ͯͨ͠Βܯࠂ warn("`App.xcodeproj/project.pbxproj` ͍ͬͯ͡Δ΍Μʁ") if git.modified_files.include? "App.xcodeproj/project.pbxproj" # Storyboard Λมߋ͍ͯͨ͠Βܯࠂ warn("`Storyboard` ͍ͬͯ͡Δ΍Μʁ") if git.modified_files.any? { |file| file.include? ".storyboard" }

No content

λʔϛφϧ্Ͱ Run: $ bundle exec danger pr https://github.com/.../.../pull/3018

Results: Warnings: - [ ] `WIP` ΍Μʁ - [ ] `500`ߦ௒͑ͱΔΘʂ - [ ] `App.xcodeproj/project.pbxproj` ͍ͬͯ͡Δ΍Μʁ - [ ] `Storyboard` ͍ͬͯ͡Δ΍Μʁ

CI ্Ͱʢαϯϓϧը૾ʣ

͍͞͝ʹ ! Stop saying "you forgot to ..." in code review by (quoted from https:/ /github.com/danger/danger/)

ࢀߟهࣄ • ΫοΫύου։ൃऀϒϩά • httpx:/ /techlife.cookpad.com/entry/2015/06/04/dokumi-ja • http:/ /techlife.cookpad.com/entry/2016/08/17/111500 • http:/ /techlife.cookpad.com/entry/2017/06/28/190000 • ϝϧΧϦ Χ΢ϧͷ։ൃؾʹͳΔʁͳΒ࿩ͦ͏ʂ by operandoOS • https:/ /speakerdeck.com/operando/merukari-kaurufalsekai-fa- qi-ninaru-narahua-sou

Q&A

Thanks!