Repository navigation
Fix "Debug artifacts after failure" PR check not checking any artifacts - #4273
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
The version list matches the upload matrix and restores existing validation without changing production behavior.
No open issues 🎉
What changed in this PR
Restores artifact validation in the “Debug artifacts after failure” PR check, which previously skipped its validation loop because VERSIONS was unset.
Changes:
- Defines
VERSIONSto match the upload job’s four-version matrix. - Enables the existing per-version and per-language artifact checks.
| File | Description |
|---|---|
.github/workflows/debug-artifacts-failure-safe.yml |
Initializes the version list used by the artifact-validation loop. |
Review effort: Max
mbg
left a comment
There was a problem hiding this comment.
Thanks for noticing this and addressing it! I wonder how this happened...
Just one optional suggestion; happy for this to be merged as-is.
| uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 | ||
| - name: Check expected artifacts exist | ||
| run: | | ||
| VERSIONS="stable-v2.20.7 default linked nightly-latest" |
There was a problem hiding this comment.
Minor: Rather than having this as a local variable here, would it be easier to spot/update if it was in an env object for the step/job/workflow?
There was a problem hiding this comment.
I'm not sure that this would be much easier to spot, and it might increase the chance of the variable being unset again.
It would also be nice to have a single source of truth for the versions, but per https://docs.github.com/en/enterprise-cloud@latest/actions/reference/workflows-and-actions/contexts#context-availability we can't use env to define a matrix and would therefore need another job to determine the matrix.
I'll leave this as is for now.
There was a problem hiding this comment.
Would it make sense to migrate these workflows to pr-checks/checks and generate them with sync.ts? Then we could perhaps move towards a single source of truth for the versions.
There was a problem hiding this comment.
Yes, now that we have validationJobs, I think it should be possible to autogenerate these workflows.
The "Check expected artifacts exist" step of the "Debug artifacts after failure" PR check loops over
$VERSIONS, but nothing in the workflow sets it. This PR setsVERSIONSto the upload job's matrix versions, as the "Debug artifact upload" check does indebug-artifacts-safe.yml.Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Environments:
How did/will you validate this change?
pr-checks).If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist