Skip to content

ci: Do not auto-approve cargo minor bumps #356

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.

Sign up for GitHub

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

Open
leongdl wants to merge 1 commit into
base: mainline
Choose a base branch
from
Open

ci: Do not auto-approve cargo minor bumps #356

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 9 additions & 0 deletions .github/workflows/auto_approve.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@ jobs:
with:
github-token: "${{ secrets.GITHUB_TOKEN }}"
- name: Approve a PR
# Hold back cargo minors and majors: the openjd-* crates are 0.x, where
# cargo treats a minor as breaking, so those most need a human to look.
# `update-type` is the highest change in the PR, so a grouped cargo PR
# containing a minor is held back too. Scoped to cargo because the pip
# and github-actions groups bundle minor with patch, so gating on patch
# alone would stop auto-approving nearly every PR from them.
if: >-
steps.metadata.outputs.package-ecosystem != 'cargo' ||

@github-actions github-actions Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The gate is scoped to cargo, but .github/dependabot.yml only configures pip and github-actions — there is no package-ecosystem: cargo entry, even though Cargo.toml / rust-bindings/Cargo.toml exist. So for Dependabot version updates this condition never fires (no cargo PRs are ever opened), and it only has an effect on Dependabot security update PRs, which are raised independently of dependabot.yml.

Not wrong, but worth being explicit about: if the intent is to actually review cargo 0.x minors, the companion change is adding a cargo ecosystem block to dependabot.yml. Otherwise a reader will assume this gate is doing more than it currently does.

steps.metadata.outputs.update-type == 'version-update:semver-patch'

@github-actions github-actions Bot Sep 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This condition still auto-approves pip and github-actions majors. Per .github/dependabot.yml, the pip-minor-patch / github-actions-minor-patch groups only match minor and patch, so majors fall outside the groups and each get their own PR with update-type == version-update:semver-major — and this if lets those straight through.

The rationale in the comment ("gating on patch alone would stop auto-approving nearly every PR from them") argues against gating on patch for those ecosystems, but it does not argue for auto-approving their majors. A major-level gate costs nothing on the grouped PRs, since those are minor+patch only and so can never report semver-major:

if: >-
  steps.metadata.outputs.update-type != 'version-update:semver-major' &&
  (steps.metadata.outputs.package-ecosystem != 'cargo' ||
   steps.metadata.outputs.update-type == 'version-update:semver-patch')

The existing black major ignore rule in dependabot.yml is evidence that pip majors here do break things (Python 3.9 compat), and today a pydantic 2→3 bump would be auto-approved.

run: gh pr review --approve "$PR_URL"
env:
PR_URL: ${{ github.event.pull_request.html_url }}
Expand Down
Loading