Miscellaneous Build/Test Tool improvements - #12935
Conversation
This description is pretty verbose and it took me 2 or 3 reads to actually understand what it was trying to say. Because the job is defined as a prerequisite for later ones with `needs`, the conditions that this one should run under are implicit, someone should be able to determine this when reading the workflow without it being spelled out explicitly.
The artifact name should always be the same, so there's no need for this input to be a string. Instead, let's make it a boolean flag with a default of `false` to avoid having to backport this change to every branch using `v3` of the reusable PHPUnit workflow. Eventually when `v4` is created, the input can go away entirely because every branch will need related the steps.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
irozum
left a comment
irozum
left a comment
There was a problem hiding this comment.
Three small, well-explained follow-ups to r62859/r62862 (per-commit rationale is genuinely helpful) — the job-name and comment cleanup in phpunit-tests.yml are straightforward and correct. I checked out the branch, ran actionlint and zizmor across .github/workflows/ (clean except pre-existing repo-wide findings unrelated to this diff), and traced the change through ticket #65721's history.
One likely blocker: the third commit changes reusable-phpunit-tests-v3.yml's gutenberg-artifact input from type: string to type: boolean, and this PR updates all five call sites in phpunit-tests.yml accordingly — but test-coverage.yml:79 is the only other caller of this reusable workflow (added in the same r62859 change per #65721) and it still passes the bare string literal gutenberg-artifact: gutenberg-build. GitHub Actions requires literal (non-expression) values for a boolean workflow_call input to be exactly true/false; a non-boolean string literal fails type validation before the run starts, so test-coverage.yml would break on its next trigger. Worth updating that call site to gutenberg-artifact: ${{ needs.prepare-gutenberg.result != 'skipped' }} (matching the pattern used in phpunit-tests.yml) in the same PR.
2 participants
Some small follow up suggestions to r62859 and r62862.
Each commit in this PR's
HEADbranch is atomic and I've included an explanation for each.Trac ticket: Core-65721
Use of AI Tools
None
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.