Skip to content

feat(reviews): filter PRs by required checks instead of all checks#1023

Closed
jd wants to merge 2 commits intomainfrom
devs/jd/worktree-review-conflict/Ie50d3888eb63c3f97907a997578a9a659dfe4ee2
Closed

feat(reviews): filter PRs by required checks instead of all checks#1023
jd wants to merge 2 commits intomainfrom
devs/jd/worktree-review-conflict/Ie50d3888eb63c3f97907a997578a9a659dfe4ee2

Conversation

@jd
Copy link
Member

@jd jd commented Mar 10, 2026

Replace the removed status:success server-side filter with a client-side
GraphQL query that checks only required status checks. This uses a
second GraphQL call with isRequired() to distinguish required from
non-required checks, so PRs are only hidden when a required check fails.

Non-required check failures (e.g. Mergify dequeue notifications) no
longer cause PRs to disappear from the review list.

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

Depends-On: #1022

@jd
Copy link
Member Author

jd commented Mar 10, 2026

This pull request is part of a stack:

  1. fix(reviews): remove status:success from default search query (#1022)
  2. feat(reviews): filter PRs by required checks instead of all checks (#1023) 👈

Copilot AI review requested due to automatic review settings March 10, 2026 20:12
@mergify mergify bot had a problem deploying to Mergify Merge Protections March 10, 2026 20:12 Failure
@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2026

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🔴 👀 Review Requirements

This rule is failing.
  • any of:
    • #approved-reviews-by>=2
    • author = dependabot[bot]
    • author = mergify-ci-bot
    • author = renovate[bot]

🔴 🔎 Reviews

This rule is failing.
  • #review-requested = 0
  • #changes-requested-reviews-by = 0
  • #review-threads-unresolved = 0

🟢 ⛓️ Depends-On Requirements

Wonderful, this rule succeeded.

Requirement based on the presence of Depends-On in the body of the pull request

🟢 🤖 Continuous Integration

Wonderful, this rule succeeded.
  • all of:
    • check-success=ci-gate

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert)(?:\(.+\))?:

🟢 📕 PR description

Wonderful, this rule succeeded.
  • body ~= (?ms:.{48,})

@mergify mergify bot requested a review from a team March 10, 2026 20:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a client-side required-checks filter to the reviews CLI so PRs are excluded only when a required check fails (instead of hiding PRs due to any failing/non-required check), replacing the previously removed status:success server-side behavior.

Changes:

  • Extend the pending reviews GraphQL search query to include PR node IDs.
  • Add a second GraphQL request that inspects required checks (isRequired()) and filters out PRs with failing required checks.
  • Update and expand CLI tests to cover required vs non-required failing checks and to account for the additional GraphQL call.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
mergify_cli/reviews/api.py Adds required-checks GraphQL query + filtering logic based on required check failures.
mergify_cli/tests/reviews/test_cli.py Updates mocks for the new two-GraphQL-call flow and adds new regression tests for required/non-required check behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

jd and others added 2 commits March 10, 2026 21:43
The status:success filter excludes PRs where any check has a non-success
state, including non-required checks like Mergify dequeue notifications.
This causes valid PRs to disappear from the review list.

Remove the server-side status filter entirely; required checks filtering
will be added client-side via GraphQL in the next commit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change-Id: Ief6ce540a050308cc8c893e8fdfecc77adff15bb
Claude-Session-Id: d5572357-9198-403e-83f3-1c3d4cb73a99
Replace the removed status:success server-side filter with a client-side
GraphQL query that checks only required status checks. This uses a
second GraphQL call with isRequired() to distinguish required from
non-required checks, so PRs are only hidden when a required check fails.

Non-required check failures (e.g. Mergify dequeue notifications) no
longer cause PRs to disappear from the review list.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Change-Id: Ie50d3888eb63c3f97907a997578a9a659dfe4ee2
Claude-Session-Id: d5572357-9198-403e-83f3-1c3d4cb73a99
@jd jd force-pushed the devs/jd/worktree-review-conflict/Ief6ce540a050308cc8c893e8fdfecc77adff15bb branch from c255b6e to d182475 Compare March 10, 2026 20:44
@jd jd force-pushed the devs/jd/worktree-review-conflict/Ie50d3888eb63c3f97907a997578a9a659dfe4ee2 branch from 0c84678 to 00a7f99 Compare March 10, 2026 20:44
@mergify mergify bot had a problem deploying to Mergify Merge Protections March 10, 2026 20:44 Failure
@mergify
Copy link
Contributor

mergify bot commented Mar 10, 2026

🧪 CI Insights

Here's what we observed from your CI run for 00a7f99.

🟢 All jobs passed!

But CI Insights is watching 👀

Base automatically changed from devs/jd/worktree-review-conflict/Ief6ce540a050308cc8c893e8fdfecc77adff15bb to main March 11, 2026 09:07
@mergify mergify bot requested a review from a team March 11, 2026 10:23
@jd jd closed this Mar 11, 2026
@jd jd deleted the devs/jd/worktree-review-conflict/Ie50d3888eb63c3f97907a997578a9a659dfe4ee2 branch March 11, 2026 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants