Upgrade to Pro — share decks privately, control downloads, hide ads and more …

Dangerを使ってPRを自動的にチェックする

 Dangerを使ってPRを自動的にチェックする

Satoshi Hachiya

August 19, 2017
Tweet

More Decks by Satoshi Hachiya

Other Decks in Programming

Transcript

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  7. Danger (Ruby)

    View full-size slide

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

    View full-size slide

  9. Danger Λಋೖ͢Δ

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  12. σϑΥϧτͷ 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

    View full-size slide

  13. λʔϛφϧ্Ͱ
    Danger Λ࢖͏

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  16. CI ্Ͱ
    Danger Λ࢖͏

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  21. 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"

    View full-size slide

  22. 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

    View full-size slide

  23. 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

    View full-size slide

  24. 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

    View full-size slide

  25. 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

    View full-size slide

  26. 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

    View full-size slide

  27. 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")

    View full-size slide

  28. 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

    View full-size slide

  29. ΫοΫύουʢ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?
    ...

    View full-size slide

  30. 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")
    ...

    View full-size slide

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

    View full-size slide

  32. iOS ༻ʹߟ͑ͯΈͨ

    View full-size slide

  33. 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"

    View full-size slide

  34. 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" }

    View full-size slide

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

    View full-size slide

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

    View full-size slide

  37. CI ্Ͱʢαϯϓϧը૾ʣ

    View full-size slide

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

    View full-size slide

  39. ࢀߟهࣄ
    • ΫοΫύου։ൃऀϒϩά
    • 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

    View full-size slide