refactor(release): collapse to 2-PR shape + delete dead rotation slice#835
Merged
bradymiller merged 4 commits intoJun 28, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR updates release automation for a two-PR conductor → docs flow, removes rotation-related tooling, and revises target ordering, command text, workflows, and test coverage to match the reduced sequence. ChangesRelease automation simplification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/release/src/ShipReleaseOrchestrator.php (1)
76-86: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winEnforce the conductor/docs target set here.
sortByMergeOrder()only rejects duplicatemergeOrdervalues. A stale caller can still pass two arbitrary PRs (or omit Docs entirely), and the orchestrator will proceed until much later. The new 2-PR flow should fail fast on the exact{Conductor, Docs}contract.🔧 Suggested fix
private function sortByMergeOrder(array $targets): array { + if (count($targets) !== 2) { + throw new \LogicException('ship-release expects exactly two targets: conductor and docs'); + } + $roles = array_map(static fn (PullRequestTarget $t): string => $t->roleLabel->value, $targets); + sort($roles); + if ($roles !== [RoleLabel::Conductor->value, RoleLabel::Docs->value]) { + throw new \LogicException('ship-release targets must be Conductor and Docs'); + } $orders = array_map(static fn (PullRequestTarget $t): int => $t->mergeOrder, $targets); if (count(array_unique($orders)) !== count($orders)) { throw new \LogicException('ship-release targets have duplicate mergeOrder values'); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/release/src/ShipReleaseOrchestrator.php` around lines 76 - 86, sortByMergeOrder() currently only validates duplicate mergeOrder values, but it also needs to enforce the exact two-target contract for the new release flow. Update ShipReleaseOrchestrator::sortByMergeOrder to reject any target set that is not precisely the expected Conductor and Docs pair before sorting, using the PullRequestTarget objects to validate both presence and identity. Keep the existing duplicate mergeOrder check, but add a fast-fail check for missing, extra, or wrong targets so stale callers cannot proceed with an invalid target list.
🧹 Nitpick comments (1)
tools/release/tests/ShipReleaseSummaryRendererTest.php (1)
27-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAlso assert the docs row in the success render test.
This fixture now exercises the full two-PR success case, but the test only checks the conductor row. A renderer regression that drops or misformats the docs row would still pass.
Suggested assertion
self::assertStringContainsString( '| conductor | `openemr/openemr` ' . '| [`#22`](https://github.com/openemr/openemr/pull/22) | ✅ merged | `def5678` |', $md, ); + self::assertStringContainsString( + '| docs | `openemr/website-openemr` ' + . '| [`#33`](https://github.com/openemr/website-openemr/pull/33) | ✅ merged | `aaa9999` |', + $md, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/release/tests/ShipReleaseSummaryRendererTest.php` around lines 27 - 43, The success render test for ShipReleaseSummaryRenderer only verifies the conductor row, so it can miss regressions in the docs entry. Update ShipReleaseSummaryRendererTest::testRenderSuccess to also assert the rendered markdown contains the docs row for the second ShipReleaseResult step, using the existing fixtures and row format alongside the current conductor assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tools/release/src/ShipReleaseOrchestrator.php`:
- Around line 76-86: sortByMergeOrder() currently only validates duplicate
mergeOrder values, but it also needs to enforce the exact two-target contract
for the new release flow. Update ShipReleaseOrchestrator::sortByMergeOrder to
reject any target set that is not precisely the expected Conductor and Docs pair
before sorting, using the PullRequestTarget objects to validate both presence
and identity. Keep the existing duplicate mergeOrder check, but add a fast-fail
check for missing, extra, or wrong targets so stale callers cannot proceed with
an invalid target list.
---
Nitpick comments:
In `@tools/release/tests/ShipReleaseSummaryRendererTest.php`:
- Around line 27-43: The success render test for ShipReleaseSummaryRenderer only
verifies the conductor row, so it can miss regressions in the docs entry. Update
ShipReleaseSummaryRendererTest::testRenderSuccess to also assert the rendered
markdown contains the docs row for the second ShipReleaseResult step, using the
existing fixtures and row format alongside the current conductor assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c439bebe-8920-416a-baaa-67aa1b25c1da
📒 Files selected for processing (29)
.github/workflows/release-permissions-check.yml.github/workflows/release-rotation.yml.github/workflows/ship-release.ymldocs/release-automation-plan.mdtools/release/README.mdtools/release/Taskfile.ymltools/release/bin/derive-slots-from-dispatch.phptools/release/bin/lint-versions.phptools/release/bin/open-rotation-pr.phptools/release/bin/rotate.phptools/release/bin/ship-release.phptools/release/src/AppPermissionProbe.phptools/release/src/LintIssue.phptools/release/src/PullRequestTarget.phptools/release/src/RoleLabel.phptools/release/src/RotationPrPublisher.phptools/release/src/ShipReleaseOrchestrator.phptools/release/src/SlotAssignmentParser.phptools/release/src/SlotRotationResult.phptools/release/src/SlotRotator.phptools/release/src/VersionsRegistryLinter.phptools/release/tests/PullRequestTargetTest.phptools/release/tests/RotationPrPublisherTest.phptools/release/tests/ShipReleaseOrchestratorTest.phptools/release/tests/ShipReleaseSummaryRendererTest.phptools/release/tests/SlotAssignmentParserTest.phptools/release/tests/SlotRotatorTest.phptools/release/tests/VersionsRegistryLinterTest.phptools/release/versions.yml
💤 Files with no reviewable changes (17)
- tools/release/versions.yml
- docs/release-automation-plan.md
- tools/release/bin/rotate.php
- .github/workflows/release-rotation.yml
- tools/release/src/LintIssue.php
- tools/release/src/RotationPrPublisher.php
- tools/release/tests/SlotRotatorTest.php
- tools/release/tests/RotationPrPublisherTest.php
- tools/release/bin/open-rotation-pr.php
- tools/release/src/VersionsRegistryLinter.php
- tools/release/tests/SlotAssignmentParserTest.php
- tools/release/src/SlotRotationResult.php
- tools/release/bin/derive-slots-from-dispatch.php
- tools/release/tests/VersionsRegistryLinterTest.php
- tools/release/src/SlotAssignmentParser.php
- tools/release/bin/lint-versions.php
- tools/release/src/SlotRotator.php
bradymiller
force-pushed
the
devops-pr1a-rotation-deletion
branch
from
47ff5e4 to
f965589
Compare
Atomic deletion of the rotation slice and collapse of ship-release.yml from 3 PRs (Infra + Conductor + Docs) to 2 PRs (Conductor + Docs). The docker-pipeline migration to openemr/openemr (openemr#790, completed 2026-06-20) moved all of rotation's live targets out of this repo. versions.yml's 13 entries all point at paths that no longer exist (docker/openemr/**, utilities/container_benchmarking/**, two deleted workflows); SlotRotator's last run on 2026-06-10 left the release-rotation/auto PR (openemr-devops#760) frozen with patches that edit files which now return 404 on master. Subsequent release-rotation.yml runs no-op because the diff against the existing branch is empty. This makes shipping today's 8.1.1 release impossible through ship-release.yml: preflight blocks on PR openemr#760's unmergeable state, or merging it would silently resurrect the deleted docker/openemr/* tree. Collapsing to 2-PR removes the Infra slot from PullRequestTarget + ShipReleaseOrchestrator entirely; PR openemr#760 becomes a dangling OPEN PR (close manually as cosmetic cleanup after this PR merges). Coordinated companion: openemr/openemr#12631 updates the cross-repo docs (RELEASE_PROCESS.md + release-automation-plan.md) to the 2-PR shape. Order: land #12631 first (docs match imminent code), then this PR (code catches up). DELETED (16 files, ~2625 LOC): - tools/release/versions.yml (registry pointing at deleted paths) - tools/release/src/SlotRotator.php, SlotAssignmentParser.php, SlotRotationResult.php, RotationPrPublisher.php, VersionsRegistryLinter.php, LintIssue.php - tools/release/bin/rotate.php, lint-versions.php, open-rotation-pr.php, derive-slots-from-dispatch.php - tools/release/tests/SlotRotatorTest.php, SlotAssignmentParserTest.php, VersionsRegistryLinterTest.php, RotationPrPublisherTest.php - .github/workflows/release-rotation.yml - docs/release-automation-plan.md (devops-side rotation slice pre-implementation design doc — misleading once the rotation code is gone) MODIFIED (13 files): - tools/release/src/PullRequestTarget.php: forRelease() drops the Infra row; Conductor mergeOrder 2->1, Docs mergeOrder 3->2; docblock updated - tools/release/src/RoleLabel.php: drop Infra enum case; docblock three->two - tools/release/src/ShipReleaseOrchestrator.php: three docblocks updated (class header, ship() targets list, sortByMergeOrder comment); no runtime code changes (orchestrator already used only RoleLabel::Conductor and RoleLabel::Docs in its logic) - tools/release/tests/PullRequestTargetTest.php: test renamed + rewritten for 2-target shape - tools/release/tests/ShipReleaseOrchestratorTest.php: full rewrite - dropped INFRA_REPO/INFRA_BRANCH constants, removed all infra fixture setup, renumbered step assertions, renamed test methods, dropped testInfraAlreadyMergedSkipsThenContinues (no 2-PR analog; SKIPPED_ALREADY_MERGED coverage preserved by testConductorAlreadyMergedRefetchesDocsBeforeMerging) - tools/release/tests/ShipReleaseSummaryRendererTest.php: dropped Infra fixture entries; first two tests now exercise Conductor rows; helper updated - tools/release/Taskfile.yml: dropped release:rotate, release:lint-versions, release:open-rotation-pr, release:derive-slots-from-dispatch, release:push-rotation-branch tasks (~80 lines); release:ship desc updated three->two - tools/release/bin/ship-release.php: header docblock + Symfony setDescription updated three->two - tools/release/README.md: dropped versions.yml row from the directory-tree diagram - tools/release/src/AppPermissionProbe.php: comment updated to drop the rotation-specific motivation (workflows:write probe still needed for other release-mechanism workflows) - .github/workflows/release-permissions-check.yml: header comment updated to drop the 'only what rotation needs' framing - .github/workflows/ship-release.yml: header comment updated for 2-PR shape NOT TOUCHED (intentional): - The cross-repo docs in openemr/openemr (covered by #12631) - Anything outside the release-mechanism + permissions-check surfaces (kubernetes, packages, raspberrypi, etc.) Tests not run locally (devops repo has no docker dev-stack); CI will exercise the test rewrites. Workstream 1 PR 1a in the release-mechanism migration plan. Assisted-by: Claude Code
…tratorTest phpcs Generic.Files.LineLength caught a 131-char line at ShipReleaseOrchestratorTest.php:323 (limit 120) introduced when rewriting the test from the 3-PR shape to 2-PR shape. Wraps the setSnapshot call across multiple lines to fit the 120-char limit. Assisted-by: Claude Code
Addresses coderabbitai outside-diff review on PR 835.
sortByMergeOrder() previously only rejected duplicate mergeOrder
values. A stale caller could still pass 3 targets (the
pre-collapse Infra+Conductor+Docs list), 1 target, or 2 targets
of the wrong role set, and the orchestrator would proceed until
much later — leading to a silent half-failure deep in the
downstream merge logic.
The 2-PR ship contract is exactly {Conductor, Docs} in any order.
sortByMergeOrder() now fails fast on:
- count(targets) != 2 — catches both pre-collapse 3-PR callers and
single-target accidents
- role set != {Conductor, Docs} — catches future role enum drift
(e.g., a new Infra-like role added without updating
PullRequestTarget::forRelease)
- duplicate mergeOrder — existing check, retained
Two new tests:
- testWrongTargetCountThrowsLogicException — covers count != 2
- testWrongTargetRoleSetThrowsLogicException — covers wrong roles
with correct count (two Conductors)
Original duplicate-mergeOrder test renamed nothing — same fixture
+ assertion, both targets are still Conductor+Docs with mergeOrder
1+1.
Assisted-by: Claude Code
bradymiller
force-pushed
the
devops-pr1a-rotation-deletion
branch
from
5ae0442 to
509afa3
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/release/src/ShipReleaseOrchestrator.php (1)
87-103: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce Conductor’s merge order before Docs.
The new contract verifies the role set, but
usort()still trusts caller-providedmergeOrder. A stale caller can pass{Conductor, Docs}with Docs order1and Conductor order2, which passes validation and merges Docs first.🐛 Proposed fix
$orders = array_map(static fn (PullRequestTarget $t): int => $t->mergeOrder, $targets); if (count(array_unique($orders)) !== count($orders)) { throw new \LogicException('ship-release targets have duplicate mergeOrder values'); } + $conductor = $this->findRequired($targets, RoleLabel::Conductor); + $docs = $this->findRequired($targets, RoleLabel::Docs); + if ($conductor->mergeOrder >= $docs->mergeOrder) { + throw new \LogicException('ship-release mergeOrder must put Conductor before Docs'); + } usort( $targets, static fn (PullRequestTarget $a, PullRequestTarget $b): int => $a->mergeOrder <=> $b->mergeOrder, );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/release/src/ShipReleaseOrchestrator.php` around lines 87 - 103, The ShipReleaseOrchestrator validation only checks that the targets are Conductor and Docs, but still sorts by caller-provided mergeOrder, so Conductor may merge after Docs. Update ShipReleaseOrchestrator to enforce the intended order explicitly when validating/sorting $targets: ensure PullRequestTarget entries with RoleLabel::Conductor always come before RoleLabel::Docs, and do not rely solely on mergeOrder for the final sequence. Keep the role-set and duplicate mergeOrder checks, but make the merge ordering deterministic based on the role labels in ShipReleaseOrchestrator.
🧹 Nitpick comments (1)
tools/release/tests/ShipReleaseOrchestratorTest.php (1)
340-381: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for reversed Conductor/Docs mergeOrder.
The validation tests cover duplicate order, wrong count, and wrong role set, but not
{Conductor, Docs}with Docs ordered before Conductor. That case should fail fast to protect the strict merge sequence.🧪 Proposed regression test
+ public function testDocsBeforeConductorMergeOrderThrowsLogicException(): void + { + $api = new FakePullRequestApi(); + $targets = [ + new PullRequestTarget('a/x', 'b1', 'master', RoleLabel::Conductor, 2), + new PullRequestTarget('a/y', 'b2', 'master', RoleLabel::Docs, 1), + ]; + + $this->expectException(\LogicException::class); + $this->expectExceptionMessage('Conductor before Docs'); + (new ShipReleaseOrchestrator($api, new FakeClock()))->ship($targets); + } +🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/release/tests/ShipReleaseOrchestratorTest.php` around lines 340 - 381, Add a regression test in ShipReleaseOrchestratorTest for the reversed merge order case where the targets are the correct {Conductor, Docs} roles but Docs has the lower mergeOrder than Conductor. Use ShipReleaseOrchestrator::ship with a FakePullRequestApi and FakeClock, and assert it throws LogicException with a message indicating the merge order/sequence is invalid. This should complement the existing testDuplicateMergeOrderThrowsLogicException, testWrongTargetCountThrowsLogicException, and testWrongTargetRoleSetThrowsLogicException coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tools/release/src/ShipReleaseOrchestrator.php`:
- Around line 87-103: The ShipReleaseOrchestrator validation only checks that
the targets are Conductor and Docs, but still sorts by caller-provided
mergeOrder, so Conductor may merge after Docs. Update ShipReleaseOrchestrator to
enforce the intended order explicitly when validating/sorting $targets: ensure
PullRequestTarget entries with RoleLabel::Conductor always come before
RoleLabel::Docs, and do not rely solely on mergeOrder for the final sequence.
Keep the role-set and duplicate mergeOrder checks, but make the merge ordering
deterministic based on the role labels in ShipReleaseOrchestrator.
---
Nitpick comments:
In `@tools/release/tests/ShipReleaseOrchestratorTest.php`:
- Around line 340-381: Add a regression test in ShipReleaseOrchestratorTest for
the reversed merge order case where the targets are the correct {Conductor,
Docs} roles but Docs has the lower mergeOrder than Conductor. Use
ShipReleaseOrchestrator::ship with a FakePullRequestApi and FakeClock, and
assert it throws LogicException with a message indicating the merge
order/sequence is invalid. This should complement the existing
testDuplicateMergeOrderThrowsLogicException,
testWrongTargetCountThrowsLogicException, and
testWrongTargetRoleSetThrowsLogicException coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8f17c348-f0b4-4695-8325-6a8b9891293b
📒 Files selected for processing (29)
.github/workflows/release-permissions-check.yml.github/workflows/release-rotation.yml.github/workflows/ship-release.ymldocs/release-automation-plan.mdtools/release/README.mdtools/release/Taskfile.ymltools/release/bin/derive-slots-from-dispatch.phptools/release/bin/lint-versions.phptools/release/bin/open-rotation-pr.phptools/release/bin/rotate.phptools/release/bin/ship-release.phptools/release/src/AppPermissionProbe.phptools/release/src/LintIssue.phptools/release/src/PullRequestTarget.phptools/release/src/RoleLabel.phptools/release/src/RotationPrPublisher.phptools/release/src/ShipReleaseOrchestrator.phptools/release/src/SlotAssignmentParser.phptools/release/src/SlotRotationResult.phptools/release/src/SlotRotator.phptools/release/src/VersionsRegistryLinter.phptools/release/tests/PullRequestTargetTest.phptools/release/tests/RotationPrPublisherTest.phptools/release/tests/ShipReleaseOrchestratorTest.phptools/release/tests/ShipReleaseSummaryRendererTest.phptools/release/tests/SlotAssignmentParserTest.phptools/release/tests/SlotRotatorTest.phptools/release/tests/VersionsRegistryLinterTest.phptools/release/versions.yml
💤 Files with no reviewable changes (17)
- tools/release/src/SlotRotationResult.php
- docs/release-automation-plan.md
- tools/release/tests/RotationPrPublisherTest.php
- tools/release/src/LintIssue.php
- tools/release/versions.yml
- tools/release/bin/lint-versions.php
- tools/release/bin/derive-slots-from-dispatch.php
- .github/workflows/release-rotation.yml
- tools/release/tests/VersionsRegistryLinterTest.php
- tools/release/bin/rotate.php
- tools/release/tests/SlotRotatorTest.php
- tools/release/tests/SlotAssignmentParserTest.php
- tools/release/src/RotationPrPublisher.php
- tools/release/src/SlotRotator.php
- tools/release/src/VersionsRegistryLinter.php
- tools/release/bin/open-rotation-pr.php
- tools/release/src/SlotAssignmentParser.php
✅ Files skipped from review due to trivial changes (5)
- tools/release/README.md
- .github/workflows/ship-release.yml
- tools/release/src/AppPermissionProbe.php
- .github/workflows/release-permissions-check.yml
- tools/release/Taskfile.yml
🚧 Files skipped from review as they are similar to previous changes (5)
- tools/release/tests/PullRequestTargetTest.php
- tools/release/bin/ship-release.php
- tools/release/src/PullRequestTarget.php
- tools/release/tests/ShipReleaseSummaryRendererTest.php
- tools/release/src/RoleLabel.php
…Order
Addresses coderabbitai outside-diff finding on PR 835 (rabbit round
2). The prior role-set + dedup-mergeOrder checks miss a case: a
stale caller could pass {Conductor: mergeOrder=2, Docs: mergeOrder=1}.
That passes the new {Conductor, Docs} contract check + the
duplicate-mergeOrder check, but usort then puts Docs first —
silently violating the strict conductor → docs merge sequence
ship-release.yml depends on.
Adds an explicit check after the dedup-mergeOrder validation:
Conductor.mergeOrder must be strictly less than Docs.mergeOrder.
Throws a LogicException with both values in the error message when
violated.
New test: testReversedMergeOrderThrowsLogicException. Verifies the
exception fires with the canonical {Conductor, Docs} role set but
reversed mergeOrder values (Conductor=2, Docs=1).
Assisted-by: Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Atomic deletion of the rotation slice and collapse of
ship-release.ymlfrom 3 PRs (Infra + Conductor + Docs) to 2 PRs (Conductor + Docs).
This PR unblocks the 8.1.1 ship. Today,
ship-release.yml's preflighteither blocks on PR #760's unmergeable state or, if forced, would merge
patches that resurrect the
docker/openemr/*tree deleted by the dockermigration. Collapsing to 2-PR removes the Infra slot entirely; PR #760
becomes a dangling OPEN PR (close manually as cosmetic cleanup after
this lands).
Why now
The docker-pipeline migration to
openemr/openemr(#790, completed2026-06-20) moved all of rotation's live targets out of this repo:
versions.yml's 13 entries all point at paths that no longer exist(
docker/openemr/**,utilities/container_benchmarking/**, twodeleted workflows)
SlotRotator's last run on 2026-06-10 leftrelease-rotation/auto(Release rotation (auto) #760) frozen with patches that edit fileswhich now return 404 on master
release-rotation.ymlruns no-op because the diff againstthe existing branch is empty
The Infra PR slot exists in
PullRequestTarget::forRelease()+ theship-release orchestrator but the underlying machinery is dead.
Coordinated companion
openemr/openemr#12631 updates the cross-repo docs
(
RELEASE_PROCESS.md+release-automation-plan.md) to the 2-PR shape.Recommended landing order: #12631 first (docs match the imminent
code state), then this PR (code catches up to the new docs).
Deleted (16 files, ~2625 LOC)
tools/release/versions.yml(registry pointing at deleted paths)tools/release/src/SlotRotator.php,SlotAssignmentParser.php,SlotRotationResult.php,RotationPrPublisher.php,VersionsRegistryLinter.php,LintIssue.phptools/release/bin/rotate.php,lint-versions.php,open-rotation-pr.php,derive-slots-from-dispatch.phptools/release/tests/SlotRotatorTest.php,SlotAssignmentParserTest.php,VersionsRegistryLinterTest.php,RotationPrPublisherTest.php.github/workflows/release-rotation.ymldocs/release-automation-plan.md(devops-side rotation slice pre-implementation design doc — misleading once the rotation code is
gone)
Modified (13 files)
PullRequestTarget.php—forRelease()drops the Infra row;Conductor
mergeOrder: 2 → 1, DocsmergeOrder: 3 → 2; docblockupdated
RoleLabel.php— dropInfraenum case; docblock "three" → "two"ShipReleaseOrchestrator.php— 3 docblocks updated; noruntime code changes (orchestrator already used only
RoleLabel::ConductorandRoleLabel::Docsin its logic)PullRequestTargetTest.php— test renamed + rewritten for2-target shape
ShipReleaseOrchestratorTest.php— full rewrite: droppedINFRA_REPO/INFRA_BRANCHconstants, removed all infra fixturesetup, renumbered step assertions, renamed test methods, dropped
testInfraAlreadyMergedSkipsThenContinues(no 2-PR analog;SKIPPED_ALREADY_MERGEDcoverage preserved bytestConductorAlreadyMergedRefetchesDocsBeforeMerging)ShipReleaseSummaryRendererTest.php— dropped Infra fixtureentries; first two tests now exercise Conductor rows
Taskfile.yml— dropped 5 tasks (release:rotate,release:lint-versions,release:open-rotation-pr,release:derive-slots-from-dispatch,release:push-rotation-branch);release:shipdesc updatedbin/ship-release.php— header docblock + SymfonysetDescriptionupdatedREADME.md— droppedversions.ymlrow from the directory-treeAppPermissionProbe.php— comment updated to drop therotation-specific motivation (workflows:write probe still needed for
build-release / release-announcements / ship-release)
release-permissions-check.yml— header comment dropped the"only what rotation needs" framing
ship-release.yml— header comment updated for 2-PR shapeWhat's NOT touched (intentional)
openemr/openemr(covered by #12631)(kubernetes, packages, raspberrypi, mariadb-backup-manager, etc.)
Test plan
gh workflow run ship-release.yml --repo openemr/openemr-devops -f version=8.1.1 -f rel_branch=rel-810proceeds through preflight (no Infra blocker)🤖 Generated with Claude Code
Summary by CodeRabbit