Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SCSZ-1622 added auto merge support #101

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions .github/workflows/dependabot-prs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,18 @@ jobs:
# a) it has the `development-dependencies` label, which we add for certain
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This dependabot setup differs from the norm. It is missing the steps:

  • Get unique committers
  • Disable auto-merge if there are commits from someone other than Dependabot
  • Add the Needs QA label to dependabots after any change by someone other than the dependabot bot

I believe I set it up like this a while back by guidance from @JacobEvelyn . IIRC, I think we omitted these because it wasn't needed. Additionally, we don't use buildkite here since the code is small enough to rely on github actions.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cyrorama if we're assigning these out it seems a little strange to me that we wouldn't disable auto-merging if someone had committed on a PR here. I'm not clear on what we gain from not doing those steps really. Any thoughts?

# categories of PRs (see `.github/dependabot.yml`), OR
# b) Dependabot has categorized it as a `direct:development` dependency,
# meaning it's in the Gemfile in a `development` or `test` group
# meaning it's in the Gemfile in a `development` or `test` group, OR
# c) our scripts have flagged the PR as an automergeable dependency (i.e
# a stable dependency with good unit test coverage) that has passed
# the waiting period.
#
# Note that we also do nothing when the PR has already had auto-merge
# enabled, to prevent scenarios where this check runs many times (for
# instance, because removing `Needs QA` triggers another run, or because
# other PRs are merging and causing this to rebase and trigger another
# run) and then approves the PR many times, which is confusing and looks
# awkward.
if: ${{ !github.event.pull_request.auto_merge && (contains(github.event.pull_request.labels.*.name, 'development-dependencies') || steps.dependabot-metadata.outputs.dependency-type == 'direct:development') }}
if: ${{ (github.actor == 'dependabot[bot]' || github.actor == 'panorama-bot-r') && steps.unique-committers.outputs.committers == '["dependabot[bot]"]' && !github.event.pull_request.auto_merge && (contains(github.event.pull_request.labels.*.name, 'development-dependencies') || steps.dependabot-metadata.outputs.dependency-type == 'direct:development' || contains(github.event.pull_request.labels.*.name, 'automerge-dependencies')) }}
run: gh pr merge --auto --merge "$PR_URL" && gh pr edit "$PR_URL" --remove-label "Needs QA" && gh pr review --approve "$PR_URL"
env:
PR_URL: ${{github.event.pull_request.html_url}}
Expand Down
Loading