Skip to content

Use latest review state in maintainer-approval#5323

Open
fallintoplace wants to merge 2 commits into
databricks:mainfrom
fallintoplace:fix-maintainer-approval-latest-review-state
Open

Use latest review state in maintainer-approval#5323
fallintoplace wants to merge 2 commits into
databricks:mainfrom
fallintoplace:fix-maintainer-approval-latest-review-state

Conversation

@fallintoplace

Copy link
Copy Markdown

Summary

  • collapse review history to each reviewer's latest state before checking approval
  • use the latest-review list for maintainer, maintainer-authored, and per-path approval checks
  • add regression tests for approve-then-changes-requested on each approval path

Why

The workflow currently treats historical APPROVED reviews as current approval. If a reviewer later switches to CHANGES_REQUESTED, the older approval can still satisfy the check.

Fixes #5322.

Test plan

  • node --test .github/scripts/owners.test.js .github/workflows/maintainer-approval.test.js

@github-actions

Copy link
Copy Markdown
Contributor

Waiting for approval

Based on git history, these people are best suited to review:

  • @simonfaltum -- recent work in .github/workflows/

Eligible reviewers: @andrewnester, @anton-107, @denik, @pietern, @renaudhartert-db, @shreyas-goenka

Suggestions based on git history. See OWNERS for ownership rules.

Comment on lines +60 to +62
// GitHub returns the full review history, so we collapse it to each user's
// latest state before evaluating whether the PR is currently approved.
function latestReviewsByUser(reviews) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this cannot just take all reviews.entries() but must only consider opinionated reviews (similar to GitHub's latestOpinionatedReviews API), i.e. APPROVED or CHANGES_REQUESTED.

Otherwise, an APPROVED followed by a COMMENTED will undo the approval. Needs to be covered by a unit test as well.

@janniklasrose janniklasrose requested a review from simonfaltum June 9, 2026 14:40
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 5323
  • Commit SHA: f9a7550a4a00e81f525817a177a315a9a890415b

Checks will be approved automatically on success.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

maintainer-approval uses historical APPROVED reviews instead of latest review state per reviewer

2 participants