Skip to content

Use latest review state in maintainer-approval#5323

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

Use latest review state in maintainer-approval#5323
fallintoplace wants to merge 1 commit 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.

@github-actions
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: 0ff58a723a2e1d8fd2a426797c21b90d4c7b37d4

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

1 participant