-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Optimize CI: Limit the number of job variations (PHP and JS matrix) in PRs #72506
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
512e1a9 to
107a4ae
Compare
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in e877574. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18667482078
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of conditionally loading the matrix values (which is pretty messy and hard to read), what if there was an event key in the matrix that contains github.event_name. Then we can just have exclude statements for macos-15 & pull_request, windows-2025 & pull_request, and then any additional versions that we'd like to exclude.
Alternatively, an if condition that only runs for certain values when the triggering event is a pull request.
We should be intentional about how we limit combinations. While having fewer jobs is better for feedback, we shouldn't just trim for the sake of it. The idea behind having multiple test combinations is that problems are caught before they are merged. Before we just take a hatchet to the matrices for each job, I think we should look at how we can make each workflow more efficient first.
I also think that for PHP, we should at a minimum test the oldest and newest versions of each major PHP version (ie. 7.2 & 7.4, 8.0 & 8.4). Testing just 8.3 is opening us up merging compatibility issues that could be fixed prior to merging.
In all the years working on Gutenberg, I don't recall a single time where I saw a Node.JS or PHP version fail but not others. I think it's pretty telling. I think we should be more aggressive here. I'm not saying that this will never happen (a version failing but not others), but for me if it ever happens and trunk is broken for a few minutes (the time it takes to resolve or revert), I think it's totally ok and better than running multiple versions on PRs for something that has a very small chance of happening. |
ellatrix
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's do this, we can discuss this further after Beta 1 and at least this will unblock a lot of PRs. In my opinion it's also much better for new contributors. We can do quick reverts and worry about PHP version numbers as committers.
priethor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed with @ellatrix , let's merge these changes to unblock Beta1 and refine after it if we need to add some specific versions. And thanks for caring about the CI
|
@desrosj I had to merge this PR to improve the situation but we can iterate on this on a calmer schedule haha |
|
I just cherry-picked this PR to the wp/latest branch to get it included in the next release: 64811f9 |
|
I just cherry-picked this PR to the release/21.9 branch to get it included in the next release: 00c8c28 |
What
Reduces the unit test matrix for pull requests to run only a single Node.js version (20) and single PHP version (8.3), while maintaining full multi-version testing on trunk and release branches.
Why
How
Uses conditional fromJSON() expressions in GitHub Actions matrix configuration based on github.event_name: