Packages: Ensure only changed packages get published for WP releases #55334
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gziolo
added
[Type] Build Tooling
Mamaduka
reviewed
Oct 13, 2023
| @@ -53,6 +53,7 @@ jobs: | |||
| with: | |||
| path: publish | |||
| ref: wp/${{ github.event.inputs.wp_version }} | |||
| fetch-depth: 100 | |||
There was a problem hiding this comment.
Let's comment here on why we need to specify fetch-depth with arbitrary number 100.
There was a problem hiding this comment.
I increased the number to 999 and used similar comments in the workflow and the Node script. We could change the strategy if someone has a better idea, but we definitely can't rely on the period of time as a factor between releases because it's very random for minor and security WordPress releases.
There was a problem hiding this comment.
The 6.2 branch compare/wp/6.2...trunk has 3,524, compare/wp/6.3...trunk has 1,682 commits already
Can this option be removed or set to 0 to force a full checkout of the repo?
What would be the impact be of setting this to 0 be?
I can only think of time to check out the full full repo would be make publishing slower.
# Number of commits to fetch. 0 indicates all history for all branches and tags.
# Default: 1
fetch-depth: ''
Or could add look into getting the branch directly by adding support for --feth-tags and get the tag e.g wp/6.2
# Whether to fetch tags, even if fetch-depth > 0.
# Default: false
fetch-tags: ''
There was a problem hiding this comment.
We create wp/X.Y branches from the last Gutenberg plugin release before Beta 1, so it's coming from release/X.Y. The very first publishing from every wp/X.Y branch by design should ignore the history as we need to publish all packages with the dist-tag wp-X.Y. In effect, we only care about subsequent bug fix releases targeting WP core which is either one commit that merges cherry-pick from trunk, or every bug fix cherry-picked individually. Definitely the number isn't so big.
What would be the impact be of setting this to 0 be?
That's always an option, but it means you need to download the entire repository consisting of:
|
Size Change: 0 B Total Size: 1.65 MB ℹ️ View Unchanged
|
|
Flaky tests detected in 8f07dfd. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/6505930889
|
ntwb
approved these changes
Oct 16, 2023
gziolo
added a commit
that referenced
this pull request
Nov 15, 2023
…55334) * Packages: Ensure only changed packages get published for WP releases * Clarify the reason for using fetch depth * Iprove the wording for comments
|
@gziolo I noticed that for example stylelint-config still gets new releases even there are no changes. Shouldn't this have stopped since this change or is this another issue? |
It would be expected for bi-weekly releases triggered at the time the Gutenberg plugin’s RC1 is created. It should not happen anymore for bug-fix releases targeting WordPress core. The biggest challenge is that we always need to force npm publishing with at least a minor version change when a new WP major release starts so we are able to publish security fixes to older versions of WP. It’s complicated, but we learned the hard way to keep that buffer. I’m open to iterating on the CI job, it’s mostly about figuring out how to detect that we need to force publishing all packages because we start preparing a new major WP release. Otherwise, we probably don’t need to publish unchanged packages. Still, Lerna will update the version if the dependencies change. For example, if |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Tries to fix the issue with publishing unmodified WordPress packages when targeting the WordPress core to avoid all the potential issues encountered yesterday during WordPress 6.1.4 and 6.3.2 releases. It also aligns with how it worked before the upgrade to Node 16 / npm 8 that forced us to stop using the Node script for npm publishing when dealing with branches that use older versions of Node & npm.
Why?
It turned out the Checkout GitHub action by default fetches only the last commit in the branch:
In effect, Lerna didn't have access to the previous commit used for npm publishing.
How?
The fix mirror the behavior that worked well in the past and it is still used for npm publishing when targeting the latest packages of the bugfix release:
gutenberg/bin/plugin/commands/packages.js
Line 103 in ef21f20
The number
100here is randomly picked based on some quick assessments from the past.