Skip to content

chore: clarify authorized admin merge handoff - #5575

Open
steve8708 wants to merge 1 commit into
mainfrom
steve8708/changes-12049-next
Open

steve8708 wants to merge 1 commit into
mainfrom
steve8708/changes-12049-next

Conversation

@steve8708

Copy link
Copy Markdown
Contributor

Summary

  • clarify that REVIEW_REQUIRED is not a user handoff under /ship
  • require the owning task to perform the guarded admin merge after the unchanged soak

Checks

  • corepack pnpm guard:workspace-skills
  • git diff --check

This change updates shipping guidance only; deployment is not part of this PR.

@builder-io-integration builder-io-integration Bot left a comment

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.

Builder reviewed your changes and has a few items to flag 🟡

Review Details

Code Review Summary

This low-risk, docs-only PR clarifies that /ship owns the authorized admin-merge handoff after the unchanged soak, rather than treating REVIEW_REQUIRED as a request for the user to click Merge. The overall direction is consistent with the existing shipping lifecycle and correctly reinforces that the owning task must complete the guarded merge.

One wording concern remains: the new review gate lists a “verified fix, reply, or terminal disposition,” which can be read as allowing an arbitrary reply to satisfy the gate even when the underlying review item is unresolved. That is weaker than the existing merge-gate language and should require an addressed/verified outcome plus any required reply, or a valid terminal disposition. This was found by one of two reviewers and is medium severity; because this is low-risk documentation, server-side filtering may omit it from inline comments.

🧪 Browser testing: Skipped — PR only modifies shipping guidance, with no UI or runtime impact.

This branch has not been deployed

No deployments
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.

1 participant