Build/Test Tools: Allow the CSS precommit task to be skipped - #13318
Build/Test Tools: Allow the CSS precommit task to be skipped#13318itzmekhokan wants to merge 1 commit into
Conversation
The `precommit:css` step added in [63315] runs Autoprefixer against whichever branch is checked out. In the 5.2 through 5.7 branches, that task has not been run since the browser usage database was last updated, so it rewrites tracked CSS files and the `git diff --exit-code` step that follows the build fails. Those branches call this reusable workflow at `@trunk`, so add a `test-css` input mirroring the existing `test-emoji` one, allowing each branch to opt out of the task. See #65993.
|
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. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
lancewillett
left a comment
lancewillett
left a comment
There was a problem hiding this comment.
This is a valid, narrowly scoped fix. I verified that the 5.2–5.7 src jobs modify tracked CSS and fail the clean-tree check.
The new input follows the existing test-emoji pattern, defaults to true, and leaves current callers unchanged. Approved for trunk.
|
@itzmekhokan, after this lands on trunk, please open one follow-up PR each against the affected branches:
In each PR, add Once all six are green, please comment here with the PR links and results. We’ll then coordinate the required second-core-committer reviews and SVN branch commits. |
The reusable build-process workflow runs `precommit:css` against the checked-out branch. On branches 5.2 through 5.7, the task rewrites tracked CSS and causes the clean-tree check to fail. Add a `test-css` input so affected branch callers can skip the task. It defaults to `true`, preserving behavior for existing callers. Developed in: #13318 Props desrosj, khokansardar. See #65993. git-svn-id: https://develop.svn.wordpress.org/trunk@63445 602fd350-edb4-49c9-b593-d223f7449a82
The reusable build-process workflow runs `precommit:css` against the checked-out branch. On branches 5.2 through 5.7, the task rewrites tracked CSS and causes the clean-tree check to fail. Add a `test-css` input so affected branch callers can skip the task. It defaults to `true`, preserving behavior for existing callers. Developed in: WordPress/wordpress-develop#13318 Props desrosj, khokansardar. See #65993. Built from https://develop.svn.wordpress.org/trunk@63445 git-svn-id: http://core.svn.wordpress.org/trunk@62626 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
Trunk fix landed in https://core.trac.wordpress.org/changeset/63445 |
|
Slack failures indicate we still need the backport PRs to land: https://wordpress.slack.com/archives/C02RQBWTW/p1788453035296919 |
|
I'm a pretty torn on this one. The fact that these CSS changes have not been committed to the branch is a bug. It's also a sign that this new step in the workflow is doing it's job and flagging legitimate problems, even ones minor in nature. While I am currently of the opinion that we should just commit the changes that were missed to these 5 or so branches instead. |
desrosj
left a comment
desrosj
left a comment
There was a problem hiding this comment.
Marking as requesting changes just so my concerns can be discussed first.
|
To make sure I was not recommending that we make changes that were too substantive, I analyzed the actual changes: 5.7 branch5.6 branch5.5 branch5.4 branch5.3 branch5.2 branchAs for the changes themselves, nearly all of the changed lines are removing In summary, the files modified combine for less than 400KB in size that would be added to the next auto-update for each, sould one be required. |
|
I’m in favor of fixing the generated CSS instead of changing the test behavior. Good catch, @desrosj. @itzmekhokan, what do you think about reverting the trunk change, keeping the test enabled on these branches, and committing the generated CSS changes instead? |
|
After more thought, I do agree we should flip this to fix CSS files instead. Suggested direction (open to more feedback!):
This fixes the code flagged by the check and preserves CI’s ability to detect future CSS drift. @itzmekhokan, since you identified the failure and contributed the original fix, I’d like you to remain part of the solution. Would you be willing to prepare the six branch PRs with the generated CSS changes? |
|
@lancewillett @desrosj — agreed, fixing the CSS is the better outcome. Six PRs are open, one per branch, each containing only the output of
All six are green. That is 36 of 36 Each branch was regenerated on Node 14 with The content matches your analysis: every deleted line is a That leaves the trunk revert of [63445], which needs a committer. Once that and the six branch commits land, https://core.trac.wordpress.org/ticket/65993 can be closed. |
|
Looking good @itzmekhokan @desrosj If you could be a 2nd reviewer on all these PRs I'll do the core commits |
|
Going to close this out as fixed because the original change was reverted. Let's use the individual PRs to follow up. |
github-project-automation
Bot
moved this from In review
to Done
in WordPress Project Build Tooling
3 participants
The
Test Build Processesworkflow now fails for the 5.2 through 5.7 branches.The
precommit:cssstep added in [63315] runs Autoprefixer against whichever branch is checked out. Those branches have not had the task run since the browser usage database was last updated, so it rewrites tracked CSS files and thegit diff --exit-codestep that follows the build then fails. Since those branches call this reusable workflow at@trunk, this adds atest-cssinput mirroring the existingtest-emojione, letting each branch opt out.It defaults to
true, so no current caller changes behaviour; the affected branches still needtest-css: falseadded alongside their existingtest-emoji: false.Trac ticket: https://core.trac.wordpress.org/ticket/65993
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Opus 5
Used for: Tracing the CI failure to the
precommit:cssstep and drafting the workflow input. All changes were reviewed and validated by me.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.