Skip to content

fix: exclude already-approved PRs from review digest #30

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

Closed
Ilia wants to merge 1 commit into from
+6 −2
Closed

fix: exclude already-approved PRs from review digest #30

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
8 changes: 6 additions & 2 deletions .github/workflows/pr-review-digest.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ name: PR Review Digest
# ${{ secrets.PACTFLOW_CI_BOT_PRIVATE_KEY }} (org secret — already exists)
#
# Posts a digest of open, non-draft, human-authored pactflow org PRs that:
# - have no reviewer assigned
# - have no reviewer assigned and no existing approval
# - have all CI checks passing
# - do not carry the "DO NOT MERGE" label
# Runs weekdays at 8:30am and 2:30pm AEST (UTC+10). Silent if nothing to report.
Expand Down Expand Up @@ -78,6 +78,9 @@ jobs:
requestedReviewer { __typename }
}
}
reviews(first: 20) {
nodes { state }
}
repository { name }
commits(last: 1) {
nodes {
Expand All @@ -100,7 +103,8 @@ jobs:
# Bot reviewers (e.g. Copilot) are intentionally ignored — a PR with
# only a bot reviewer still needs a human to pick it up.
([.reviewRequests.nodes[] | select(.requestedReviewer.__typename == "User" or .requestedReviewer.__typename == "Team")] | length) == 0 and

@JP-Ellis JP-Ellis Jun 9, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Ah, I didn't see your update in your last PR.

So for the purposes of this notification, we ignore PRs that have a human review request, but still need a review? Wouldn't it be simpler to just disregard the number of requests entirely? Afterall, the notification is just for "PR needing a review" (which would imply irrespective of how many review requests it has)

(.commits.nodes[0].commit.statusCheckRollup.state == "SUCCESS")
(.commits.nodes[0].commit.statusCheckRollup.state == "SUCCESS") and
(.reviews.nodes | map(select(.state == "APPROVED")) | length) == 0

@JP-Ellis JP-Ellis Jun 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this will work for PRs in repos configured to dismiss reviews if there's a subsequent change to the PR, which then means that the PR won't appear in the notification for re-review.

) |
. + { slack_id: $mapping[.author.login].slack_id }
] | sort_by(.createdAt)
Expand Down
Loading