Repository navigation
Update TheRock reference to (2a55cec) - #13623
Conversation
|
🤖 Bump PR detected (author |
CI review: tracked breakdownVerdict: safe to merge. This bump does not add CI red beyond what is already tracked or newly filed. Both failures are Windows gfx110X quick-tier timeouts that also fail on develop with the old pin, so they are not caused by the bump. As usual Per-failure disposition (rocm-libraries scope only)
The two What this bump actually brings inTheRock
Writes made in this review
Caveat: the nightly baseline could not be computed this round (the jobs API returned 502 for the 10-13 nightly), but Windows gfx110X has not been in the nightly matrix on earlier reviews, so it would likely not have added information. |
tony-davis
left a comment
There was a problem hiding this comment.
Approving after CI review.
The bot cleared baseline-run-id in #13623 alongside the TheRock ref bump to 2a55cec, and this branch inherited the empty value when develop was merged. An empty baseline makes therock-multi-arch-ci.yml pass prebuilt_stages="" and stage_reuse_mode=off, so every stage rebuilds and the run looks like changed_projects failed to narrow anything. It did not: setup resolved all 34 rocm-libraries projects and put rccl/rocshmem/core-hiptests in reusable_artifacts, exactly as #8514 expects. Restore 38220501023, the baseline develop carried before the bump and the one the stacked rocPRIM PR still uses, so the two test PRs differ only in their diffs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| baseline-run-id: | ||
| description: "TheRock workflow run ID for prebuilt stage artifacts" | ||
| value: "38220501023" # Updated by assistant-librarian[bot] on submodule bumps | ||
| value: "" # Updated by assistant-librarian[bot] on submodule bumps |
There was a problem hiding this comment.
Dropping the baseline-run-id here is causing lots of workflow runs to rebuild all stages, including the WSL ROCDXG stage that uses very limited WSL runners. This is contributing to job queue times across repositories.
There was a problem hiding this comment.
For example: https://github.com/ROCm/rocm-libraries/actions/runs/38633953407?pr=13671 built the "emulation", "storage libs", "debug tools", "runtime tests", "comm libs", "WSL ROCXG", and other stages that are independent of rocm-libraries.
…ernal-ci-file-goodbye Both conflicts trace to #13698, which reverted the TheRock ref bump #13623. .github/workflows/therock-multi-arch-ci.yml: kept this branch's version. Every develop hunk re-pins an @<ref> from 2a55cec to c01e3a3, and this branch deliberately points those at the users/geomin12/external-ci-file-goodbye branch instead. The remaining hunk restores the `configure` job and its configure_external_repo_ci.py call, which is exactly what this PR deletes. .github/actions/ci-env/action.yml: took develop's version. The revert restored baseline-run-id 38220501023, the same value 67a16e4 pinned by hand, so the stage reuse this branch needs survives while the now-stale DO-NOT-MERGE rationale goes away: develop owns that value again. therock-ref follows develop back to c01e3a3; its only consumer here is the rocjitsu race check job. Side effect: ci-env now matches develop exactly, so it drops out of the PR diff and the run narrows on the workflow path alone.
TheRock Submodule Bump ActivityNewest first
|
…d scripts (#13729) Resolves #13727 ## Summary Adds a `github-ci-pr-quality` overlay on the `rocm-pr-quality` base skill for changes under `.github/`. It tells authors and reviewers where CI tests belong (extend an existing lane, not a new workflow per test), how to size runners and checkout, how to treat shared state in `ci-env`, and to read each affected project's TESTING.md before changing a lane or gate. It also covers what a CI job checks out (the PR merge ref, `refs/pull/<N>/merge`, so every lane tests the same tree) and requires CI references to be updated when files move or are deleted. A matching Copilot review instructions file loads it for `.github/**`. This is the second of two PRs; it is stacked on the root AGENTS.md PR (#13728), so its base is that branch until #13728 merges. ## Risk Assessment 1/5. Guidance for reviewers and AI assistants only. No workflow, script, or build changes, and the guidance is advisory. One behavior change: Copilot code review will start applying this guidance to PRs that touch `.github/**`. ## Related - Work tracking: #13727 - Depends on: #13728 (the root AGENTS.md PR, tracked in #13726) - Cases it is designed to catch: #12185, #13623 (reverted in #13698), and the stale paths left by #12150 (fixed in #13399) - Review input: a skill-forge review of the PR-quality skills (findings R2, R3, R4, R8, M4/R7 applied here; the helper findings M1, M2, M3, and R5 are fixed in ROCm/TheRock#6653; the rest of the base-skill findings are still open) - Base skill: `skills/rocm-pr-quality/` in ROCm/TheRock ## Testing Summary - `pre-commit` on the changed files passed. Every workflow, script, action value, and path the skill cites was checked against `origin/develop`. ## Testing Checklist - [x] pre-commit on the changed files - Status: Passed - [x] Cited paths and facts checked against `develop` - Status: Passed - [ ] PR CI - GitHub PR checks - Status: Pending ## Flags / Guardrails None. The guidance is advisory. ## Adjacent Tests Considered None apply; no code changes. ## Technical Changes - New `skills/github-ci-pr-quality/SKILL.md` with the TESTING.md cross-read, five CI change classes, and test-placement, runner-sizing, trigger-scope, tested-tree, and shared-state rules. Trigger scope includes checks that path filters match real files and that moves and deletes update CI in the same PR. It tightens stale-base and approvals, adds a review checklist, three worked examples, and a completed retrospective review of #12185 as a model of the required output. - Review contract: the overlay keeps review read-only (pre-commit runs in a throwaway checkout), and requires caller edits only for breaking workflow interface changes. - New `.github/instructions/github-ci-pr-quality.instructions.md` (`applyTo: ".github/**"`). - `AGENTS.md`: the planned catalog row now links to the skill, and the new instructions file is listed. ## Risk Acceptance / Waivers `W-DOC`: guidance only; no product or CI behavior changes. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
## Motivation The current develop branch uses TheRock commit 4743438 with baseline run 38722718475. This pull request moves the pin from the previous pair, d28f23b / 39269411010, to the current rocm-libraries bump and its matching baseline. ## Technical Details - Updates the pinned TheRock commit: From: 47434387de730d51788b705e5600602dd6064997 To: b0958085a97cb03a2c38931b8e2ec86ac78d1563 - Updates baseline-run-id: From: 38722718475 To: 39513854305 - The baseline was produced from the same b0958085 TheRock commit, TheRock #9359: https://github.com/ROCm/TheRock/actions/runs/39513854305 REF: #13623, #13698 ## Test Plan ## Test Result ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/TheRock/blob/main/GOVERNANCE.md#pull-requests. --------- Co-authored-by: Cursor <cursoragent@cursor.com>
Updated all pinned TheRock workflow and source refs to
2a55cecdue to submodule bump