diff --git a/.github/workflows/check_review_status.yml b/.github/workflows/check_review_status.yml deleted file mode 100644 index 56d791f26..000000000 --- a/.github/workflows/check_review_status.yml +++ /dev/null @@ -1,35 +0,0 @@ -name: 'Check review status' - -on: - pull_request_review: - types: [submitted, edited, dismissed] - -jobs: - check_review_status: - runs-on: 'ubuntu-latest' - permissions: - pull-requests: read - contents: read - env: - BASE_SHA: ${{ github.event.pull_request.base.sha }} - HEAD_SHA: ${{ github.event.pull_request.head.sha }} - PR_AUTHOR: ${{ github.event.pull_request.user.login }} - GH_REPO: ${{ github.repository }} - steps: - - uses: actions/checkout@v4 - with: - filter: 'blob:none' - fetch-depth: 0 - ref: main - - name: Changed gems and non gem files - id: changes - run: | - ruby .github/workflows/pr_bot/changed_files.rb - - name: Check review status - env: - CHANGED_GEMS: ${{ steps.changes.outputs.gems }} - CHANGED_NON_GEMS: ${{ steps.changes.outputs.non_gems }} - PR_NUMBER: ${{ github.event.pull_request.number }} - GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - ruby .github/workflows/pr_bot/check_review_status.rb "$CHANGED_GEMS" "$CHANGED_NON_GEMS" "$PR_NUMBER" diff --git a/.github/workflows/comment_on_review.yml b/.github/workflows/comment_on_review.yml new file mode 100644 index 000000000..6eec4b143 --- /dev/null +++ b/.github/workflows/comment_on_review.yml @@ -0,0 +1,29 @@ +name: Comment on review + +on: + workflow_run: + workflows: [Generate comment body on review] + types: [completed] + + +jobs: + comment: + runs-on: ubuntu-latest + if: ${{ github.event.workflow_run.conclusion == 'success' }} + permissions: + pull-requests: write + actions: read + steps: + - name: Download artifact + uses: actions/download-artifact@v4 + with: + name: comment_body_on_review + run-id: ${{ github.event.workflow_run.id}} + github-token: ${{ secrets.GITHUB_TOKEN }} + - name: Send a comment + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + run: + gh pr comment "$(cat PR_NUMBER)" --body "$(cat COMMENT_BODY.txt)" --repo "$GH_REPO" + diff --git a/.github/workflows/generate_comment_body_on_review.yml b/.github/workflows/generate_comment_body_on_review.yml new file mode 100644 index 000000000..58324a1c3 --- /dev/null +++ b/.github/workflows/generate_comment_body_on_review.yml @@ -0,0 +1,56 @@ +# This workflow generates a comment body on an approval. +# It generates the body and saves it as an artifact. +# A following workflow called from `workflow_run` event uses the artifact. +# +# These workflows are separated due to GITHUB_TOKEN's permission. +# GitHub does not allow `write` permission on public fork repositories on `pull_request_review` event. +# Therefore we need to use `workflow_run` event to permit `write` actions to the token. + +name: 'Generate comment body on review' + +on: + pull_request_review: + types: [submitted] + +jobs: + create_comment: + if: github.event.review.state != 'CHANGES_REQUESTED' + runs-on: 'ubuntu-latest' + env: + BASE_SHA: ${{ github.event.pull_request.base.sha }} + HEAD_SHA: ${{ github.event.pull_request.head.sha }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + GH_REPO: ${{ github.repository }} + steps: + - uses: actions/checkout@v4 + with: + filter: 'blob:none' + fetch-depth: 0 + ref: main + - uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.3' + bundler-cache: true + - name: Changed gems and non gem files + id: changes + run: | + ruby .github/workflows/pr_bot/changed_files.rb + - name: Prepare comment body + id: comment-body + env: + CHANGED_GEMS: ${{ steps.changes.outputs.gems }} + CHANGED_NON_GEMS: ${{ steps.changes.outputs.non_gems }} + PR_NUMBER: ${{ github.event.pull_request.number }} + REVIEWED_BY: ${{ github.event.review.user.login }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + run: | + ruby .github/workflows/pr_bot/review_result_comment_body.rb "$CHANGED_GEMS" "$CHANGED_NON_GEMS" "$PR_NUMBER" COMMENT_BODY.txt + echo "$PR_NUMBER" > PR_NUMBER + - name: Store comment body + uses: actions/upload-artifact@v4 + with: + name: comment_body_on_review + path: | + COMMENT_BODY.txt + PR_NUMBER + retention-days: 1 diff --git a/.github/workflows/merge_on_comment.yml b/.github/workflows/merge_on_comment.yml index ad48bfe9d..8bd2703d4 100644 --- a/.github/workflows/merge_on_comment.yml +++ b/.github/workflows/merge_on_comment.yml @@ -5,8 +5,24 @@ on: types: [created] jobs: + is_merge_command: + if: ${{ github.event.issue.pull_request }} + runs-on: 'ubuntu-latest' + outputs: + is_merge_command: ${{ steps.set_is_merge_command.outputs.is_merge_command }} + steps: + - name: Set is_merge_command + id: set_is_merge_command + env: + COMMENT_BODY: ${{ github.event.comment.body }} + run: | + ruby -e ' + is_merge_command = ENV["COMMENT_BODY"].match?(%r!\A/merge\s*\z!) + puts "is_merge_command=#{is_merge_command}" + ' >> $GITHUB_OUTPUT merge: - if: ${{ github.event.issue.pull_request && github.event.comment.body == '/merge' }} + needs: is_merge_command + if: ${{ needs.is_merge_command.outputs.is_merge_command == 'true' }} runs-on: 'ubuntu-latest' permissions: pull-requests: write @@ -17,6 +33,10 @@ jobs: filter: 'blob:none' fetch-depth: 0 ref: main + - uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.3' + bundler-cache: true - name: Fetch PR information id: pr_info env: @@ -45,11 +65,12 @@ jobs: HEAD_SHA: ${{ steps.pr_info.outputs.head_sha }} PR_AUTHOR: ${{ steps.pr_info.outputs.author }} CHANGED_GEMS: ${{ steps.changes.outputs.gems }} + CHANGED_NON_GEMS: ${{ steps.changes.outputs.non_gems }} - COMMENTED_BY: ${{ github.event.issue.user.login }} + COMMENTED_BY: ${{ github.event.comment.user.login }} PR_NUMBER: ${{ github.event.issue.number }} GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} GH_REPO: ${{ github.repository }} run: | - ruby .github/workflows/pr_bot/check_merge_ability.rb "$CHANGED_GEMS" - gh pr merge "$PR_NUMBER" --repo "$GH_REPO" --merge + ruby .github/workflows/pr_bot/check_merge_ability.rb "$CHANGED_GEMS" "$CHANGED_NON_GEMS" "$PR_NUMBER" + gh pr merge "$PR_NUMBER" --repo "$GH_REPO" --squash diff --git a/.github/workflows/pr_bot/changed_files.rb b/.github/workflows/pr_bot/changed_files.rb index 6ce1a7809..ccf9cca6e 100644 --- a/.github/workflows/pr_bot/changed_files.rb +++ b/.github/workflows/pr_bot/changed_files.rb @@ -1,6 +1,6 @@ require_relative "./utils" -paths = `git diff --name-only -z #{BASE_SHA} #{HEAD_SHA}`.split("\0") +paths = `git diff --name-only -z #{BASE_SHA}...#{HEAD_SHA}`.split("\0") changed_gems = paths.select { _1.start_with?("gems/") }.map { _1.split("/")[1] }.uniq changed_non_gems = paths.reject { _1.start_with?("gems/") } diff --git a/.github/workflows/pr_bot/check_merge_ability.rb b/.github/workflows/pr_bot/check_merge_ability.rb index dd43037fd..36e70ec67 100644 --- a/.github/workflows/pr_bot/check_merge_ability.rb +++ b/.github/workflows/pr_bot/check_merge_ability.rb @@ -1,17 +1,34 @@ require_relative "./utils" +require_relative "./merge_ability" -def can_merge?(commented_by, author, gem_reviewers) - return true if commented_by == author - return true if administorators.include?(commented_by) - return true if gem_reviewers.include?(commented_by) - return false +def comment_to_github(body, pr_number) + sh! 'gh', 'pr', 'comment', pr_number, '--body', body, '--repo', GH_REPO end -def all_gem_reviewers(changed_gems) - changed_gems.flat_map { |gem| gem_reviewers(gem, BASE_SHA) } +changed_gems = JSON.parse(ARGV[0]) +changed_non_gems = JSON.parse(ARGV[1]) +pr_number = ARGV[2] + +ability = MergeAbility.new(changed_gems:, changed_non_gems:, pr_number:) + +unless ability.can_merge_by?(ENV['COMMENTED_BY'], PR_AUTHOR) + body = <<~BODY + `/merge` command failed. + + You do not have permission to merge this PR. + PR author, reviewers, and administrators can merge this PR. + BODY + comment_to_github(body, pr_number) + exit 1 end -reviewers = all_gem_reviewers(JSON.parse(ARGV[0])) -return if can_merge?(ENV['COMMENTED_BY'], PR_AUTHOR, reviewers) +unless ability.approved? + body = <<~BODY + `/merge` command failed. -raise "You do not have permission to merge this PR." + This PR is not approved yet by the reviewers. Please get approval from the reviewers. + See the Actions tab for detail. + BODY + comment_to_github(body, pr_number) + exit 1 +end diff --git a/.github/workflows/pr_bot/check_review_status.rb b/.github/workflows/pr_bot/check_review_status.rb deleted file mode 100644 index 8785d2695..000000000 --- a/.github/workflows/pr_bot/check_review_status.rb +++ /dev/null @@ -1,49 +0,0 @@ -require_relative "./utils" -require "json" - -def gem_accepted?(gem, approvers) - reviewers = gem_reviewers(gem, BASE_SHA) - - # If reviewers is empty, it means that anyone cannot approve this PR. - # So, we can merge this PR without approval. - return true if reviewers.empty? - - # If the author is a reviewer, they can merge this PR themselves. - return true if reviewers.include?(PR_AUTHOR) - - not (reviewers & approvers).empty? -end - -def non_gem_accepted?(approvers) - admins = administorators.map { _1['login'] } - - not (approvers & admins).empty? -end - -def main(changed_gems, changed_non_gems, pr_number) - approvers = approvements(HEAD_SHA, pr_number).map { _1['user']['login'] } - - status = 0 - - # Check gem files - not_approved_gems = changed_gems.reject { |gem| gem_accepted?(gem, approvers) } - unless not_approved_gems.empty? - puts "The following gems are not approved yet:" - puts not_approved_gems.join("\n") - status = 1 - end - - # Check non gem files - if !changed_non_gems.empty? && !non_gem_accepted?(approvers) - puts "The following files are changed, but not approved by the admin yet:" - puts changed_non_gems.join("\n") - status = 1 - end - - exit status -end - -changed_gems = JSON.parse(ARGV[0]) -changed_non_gems = JSON.parse(ARGV[1]) -pr_number = ARGV[2] -main(changed_gems, changed_non_gems, pr_number) diff --git a/.github/workflows/pr_bot/merge_ability.rb b/.github/workflows/pr_bot/merge_ability.rb new file mode 100644 index 000000000..86c8b2a53 --- /dev/null +++ b/.github/workflows/pr_bot/merge_ability.rb @@ -0,0 +1,72 @@ +require_relative "./utils" + +class MergeAbility + attr_reader :changed_gems, :changed_non_gems, :pr_number + + def initialize(changed_gems:, changed_non_gems:, pr_number:) + @changed_gems = changed_gems + @changed_non_gems = changed_non_gems + @pr_number = pr_number + end + + def approved? + is_approved = true + + # Check gem files + unless not_approved_gems.empty? + log "The following gems are not approved yet:" + log not_approved_gems.join("\n") + is_approved = false + end + + # Check non gem files + if waiting_admin_approval? + log "The following files are changed, but not approved by the admin yet:" + log changed_non_gems.join("\n") + is_approved = false + end + + is_approved + end + + def can_merge_by?(commented_by, author) + return true if commented_by == author + return true if administorators.include?(commented_by) + return true if all_gem_reviewers.include?(commented_by) + return false + end + + def not_approved_gems + @not_approved_gems ||= changed_gems.reject { |gem| gem_accepted?(gem) } + end + + def waiting_admin_approval? + return false if changed_non_gems.empty? + + admins = administorators.map { _1['login'] } + (approvers & admins).empty? + end + + private + + def all_gem_reviewers + @all_gem_reviewers ||= changed_gems.flat_map { |gem| gem_reviewers(gem, BASE_SHA) } + end + + def gem_accepted?(gem) + reviewers = gem_reviewers(gem, BASE_SHA) + + # If reviewers is empty, it means that anyone cannot approve this PR. + # So, we can merge this PR without approval. + return true if reviewers.empty? + + # If the author is a reviewer, they can merge this PR themselves. + return true if reviewers.include?(PR_AUTHOR) + + not (reviewers & approvers).empty? + end + + def approvers + @approvers ||= approvements(HEAD_SHA, pr_number).map { _1['user']['login'] } + end +end diff --git a/.github/workflows/pr_bot/review_result_comment_body.rb b/.github/workflows/pr_bot/review_result_comment_body.rb new file mode 100644 index 000000000..1002b3253 --- /dev/null +++ b/.github/workflows/pr_bot/review_result_comment_body.rb @@ -0,0 +1,35 @@ +require_relative "./merge_ability" + +changed_gems = JSON.parse(ARGV[0]) +changed_non_gems = JSON.parse(ARGV[1]) +pr_number = ARGV[2] +file = ARGV[3] + +ability = MergeAbility.new(changed_gems:, changed_non_gems:, pr_number:) + +reviewer = ENV['REVIEWED_BY'] + +msg = "Thanks for your review, @#{reviewer}!\n\n" + +if ability.approved? + msg << <<~MSG + @#{PR_AUTHOR}, @#{reviewer} This PR is ready to be merged. + Just comment `/merge` to merge this PR. + MSG +else + unless ability.not_approved_gems.empty? + msg << <<~MSG + This PR still needs approval from the reviewers of the following gems. + #{ability.not_approved_gems.map { "* `#{_1}`" }.join("\n")} + MSG + end + if ability.waiting_admin_approval? + msg << <<~MSG + This PR still needs approval from the administrators. + MSG + end + + msg << "Please get approval from the reviewers." +end + +File.write(file, msg) diff --git a/.github/workflows/pr_bot/utils.rb b/.github/workflows/pr_bot/utils.rb index fb273ae27..071c1be95 100644 --- a/.github/workflows/pr_bot/utils.rb +++ b/.github/workflows/pr_bot/utils.rb @@ -63,7 +63,12 @@ def gh_api!(*args) def approvements(sha, pr_number) reviews = gh_api! "/repos/#{GH_REPO}/pulls/#{pr_number}/reviews" - reviews.select { _1['state'] == 'APPROVED' && _1['commit_id'] == sha } + reviews.select do + _1['commit_id'] == sha && ( + _1['state'] == 'APPROVED' || + (_1['body'].include?('APPROVE') && _1['state'] == 'COMMENTED') + ) + end end def administorators diff --git a/.github/workflows/pr_bot/welcome_comment_body.rb b/.github/workflows/pr_bot/welcome_comment_body.rb index 9424d8afe..cd9d7ddd2 100644 --- a/.github/workflows/pr_bot/welcome_comment_body.rb +++ b/.github/workflows/pr_bot/welcome_comment_body.rb @@ -55,7 +55,8 @@ msg << <<~MSG You need to get approval from the reviewers of this gem. - #{reviewers.map { "@#{_1}" }.join(', ')}, please review and approve the changes. + #{reviewers.map { "@#{_1}" }.join(', ')}, please review this pull request. + If this change is acceptable, please make a review comment including `APPROVE`. After that, the PR author or the reviewers can merge this PR. Just comment `/merge` to merge this PR. MSG @@ -76,7 +77,8 @@ msg << <<~MSG You need to get approval from the reviewers of this gem. - #{reviewers.map { "@#{_1}" }.join(', ')}, please review and approve the changes. + #{reviewers.map { "@#{_1}" }.join(', ')}, please review this pull request. + If this change is acceptable, please make a review comment including `APPROVE`. After that, the PR author or the reviewers can merge this PR. Just comment `/merge` to merge this PR. MSG diff --git a/.github/workflows/welcome_comment.yml b/.github/workflows/welcome_comment.yml index 242842e13..9251ba2a2 100644 --- a/.github/workflows/welcome_comment.yml +++ b/.github/workflows/welcome_comment.yml @@ -1,7 +1,7 @@ name: 'Test PR comment' on: - pull_request: + pull_request_target: types: [opened] jobs: @@ -21,10 +21,13 @@ jobs: filter: 'blob:none' fetch-depth: 0 ref: main + - uses: ruby/setup-ruby@v1 + with: + ruby-version: '3.3' + bundler-cache: true - name: Changed gems and non gem files id: changes run: | - # TODO: checkout the main branch ruby .github/workflows/pr_bot/changed_files.rb - name: Prepare comment body id: comment-body @@ -33,11 +36,10 @@ jobs: CHANGED_NON_GEMS: ${{ steps.changes.outputs.non_gems }} run: | ruby .github/workflows/pr_bot/welcome_comment_body.rb "$CHANGED_GEMS" "$CHANGED_NON_GEMS" - - name: test message + - name: Send a comment env: PR_NUMBER: ${{ github.event.pull_request.number }} GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GH_REPO: ${{ github.repository }} COMMENT_BODY: ${{ steps.comment-body.outputs.welcome_comment_body }} run: gh pr comment "$PR_NUMBER" --body "$COMMENT_BODY" --repo "$GH_REPO"