Conversation
If the published commit-ish was built and tested before, reuse that build rather than making a new build. This saves time, but more importantly it reduces the possibility of introducing new issues through a rebuild.
There was a problem hiding this comment.
Pull request overview
Updates the release publishing automation to publish the same build outputs that were produced (and intended to be) validated by CI, with a fallback build path when no suitable CI artifacts are available, plus several GitHub Actions cleanups.
Changes:
- Add a “find CI run + download artifacts” flow to
publish.yml, with a fallback to running the reusableci.ymlbuild when needed. - Extend
ci.ymlto support aforce-full-buildinput and refine package filtering/test skipping behavior; retain build artifacts longer. - Misc. workflow hygiene: concurrency controls and small composite-action cleanups.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/publish.yml | Looks up CI run artifacts for the release target and publishes using downloaded artifacts, with fallback build path. |
| .github/workflows/ci.yml | Adds force-full-build input, refines changed-package resolution, and uploads build artifacts with longer retention. |
| .github/workflows/ghpages-cleanup.yml | Adds concurrency to avoid overlapping cleanups. |
| .github/workflows/commitlint.yml | Cancels in-progress runs for the same PR/ref via concurrency. |
| .github/actions/test-package/action.yml | Simplifies Playwright install to a step-level conditional. |
| .github/actions/install-dependencies/action.yml | Minor cleanup (remove redundant working directory). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Test report for task-herder28 tests 28 ✅ 0s ⏱️ Results for commit 9c4f407. ♻️ This comment has been updated with latest results. |
Test report for scratch-svg-renderer 1 files 60 suites 0s ⏱️ Results for commit 9c4f407. ♻️ This comment has been updated with latest results. |
Test report for scratch-render 1 files 55 suites 3s ⏱️ Results for commit 9c4f407. ♻️ This comment has been updated with latest results. |
Test report for scratch-gui 2 files 62 suites 10m 54s ⏱️ Results for commit 9c4f407. ♻️ This comment has been updated with latest results. |
Test report for scratch-vm 1 files 886 suites 1m 57s ⏱️ Results for commit 9c4f407. ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Pull request overview
Updates the release publishing automation so npm publish reuses the exact build artifacts produced and tested by CI, reducing the chance of drift between “tested” and “published” outputs, along with several GitHub Actions cleanups/refinements.
Changes:
- Add
prepublishOnlyguards to prevent local/manual publishes outside CI. - Rework
publish.ymlto locate and download prior CI build artifacts for the release target (with a fallback full CI build when artifacts don’t exist). - Refine
ci.ymlpackage filtering/skip logic and tighten up workflow/action behavior (concurrency, conditional steps, artifact retention).
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/task-herder/package.json | Adds prepublishOnly to block non-CI publishes. |
| packages/scratch-vm/package.json | Replaces prepublish with prepublishOnly to block non-CI publishes. |
| packages/scratch-svg-renderer/package.json | Adds prepublishOnly to block non-CI publishes. |
| packages/scratch-render/package.json | Replaces prepublish with prepublishOnly to block non-CI publishes. |
| packages/scratch-gui/package.json | Adds prepublishOnly to block non-CI publishes. |
| .github/workflows/publish.yml | Finds/uses tested CI build artifacts for publishing; adds fallback build; adds release comments. |
| .github/workflows/ghpages-cleanup.yml | Adds concurrency grouping to avoid overlapping cleanup runs. |
| .github/workflows/commitlint.yml | Cancels in-progress runs for the same PR/head to reduce CI load. |
| .github/workflows/ci.yml | Adds force-full-build input, refines package-change resolution, and adjusts when test/results run; sets artifact retention. |
| .github/actions/test-package/action.yml | Moves Playwright install gating to step-level if. |
| .github/actions/install-dependencies/action.yml | Minor cleanup (removes redundant working-directory). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
If there's some sort of complex emergency where publishing locally is necessary, use `--ignore-scripts` to skip the `prepublishOnly` protection.
993c36e to
9c4f407
Compare
Proposed Changes
Main change: make sure that the publish workflow uses the same build output that was tested.
Supporting changes: lots of GHA cleanup & refinement.
Reason for Changes
This removes the possibility of introducing new issues between the build-and-test in the CI workflow and the build-and-publish of the publishing workflow. In theory, those two builds should be the same, but we couldn't be certain of that before. By publishing the existing build artifacts, we can now be certain. It also saves some time in the publishing workflow. In the event that we publish a commit-ish for which no build is available - for example, if we publish a commit that affected only docs, or only files in
.github/- then fall back to building everything as part of the publishing workflow.The rest of the changes are more of a "while I'm in here" set of things, including:
env:for these reasons. Not all of the values are vulnerable to that, but I'd like to establish it as a habit.testandresultsif no packages have been modifiedTest Coverage
I tested as much as I could locally by running shell commands in a terminal, etc., but the real test will be when this is merged and the first time we publish a new release after that.