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.
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
Update the npm packages release process #14136
Update the npm packages release process #14136
Changes from 1 commit
d74b326
2b792bc
2d83385
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Currently the https://github.com/wordpress/wordpress-develop repo uses
x.y
for branches, andx.y.z
for tags, this includes the major releases tagged asx.y.z
, for example5.1.0
• https://github.com/WordPress/wordpress-develop/branches
• https://github.com/WordPress/wordpress-develop/tags
I suggest rather than
wp/5.2
thatwp/5.2.0
is used so that Gutenberg uses a consistent 3 digit nomenclature?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.
I think Gutenberg should use the same thing
wp/5.2
for the branch andx.y.z
for tags. thewp/5.2
will serve for all5.2.1
,5.2.2
...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.
When WordPress releases a major release it is both branched and tagged, so a
5.0
branch is created, and a5.1.0
tag is created:•
5.0
branch https://github.com/WordPress/wordpress-develop/tree/5.1•
5.1.0
tag https://github.com/WordPress/wordpress-develop/tree/5.1.0All
package.json
files use the 3 digit nomenclature:https://github.com/WordPress/wordpress-develop/blob/5.1.0/package.json#L3:
Also, to note that the
5.0
branch has now had 4 minor releases so the 5.0 branchpackage.json
file is now5.0.4
:https://github.com/WordPress/wordpress-develop/blob/5.0/package.json#L3:
If as you suggest, Gutenberg only use
wp/5.2
for all5.2.x
releases I think that would work and be less work to maintain....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.
Tags are automatically created by Lerna when we publish the packages. we can probably also tag
wp-5.2.1
ourselves, but the issue is that at the time of the npm release, we don't know yet if it's the final npm release that will be merged in this WP version and if we won't need more. So maybe just using the branch and the lerna tags is enough.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.
Should this note be moved after the list?
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.
I prefer to keep it here, as it's important to read and stop the process if
trunk
is not open yet.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.
I see this as troublesome, not all packages get published during the package release process and I'm not sure what the point of bumping the minor version in
major.minor.patch
achieves here.Also, as we're hoping for wider spread usage from both inside and outside of the WordPress ecosystem bumping the minor version for no apparent reason is not very semantic version friendly.
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.
The idea is that there will be potential bug fixes and security fixes we're not aware and we'll leave room for it. In general a Gutenberg release means enhancements... and not everything is mentioned in the CHANGELOG.
With our lerna process, packages will be meant to be published without even touching them, if we change one of their dependencies.
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.
We're struggling because of this at the moment, we can't update trunk because there might be a 5.1.1 for WordPress requiring security fixes or bug fixes we're not aware of. Right now we default to patch when we don't know and it's blocking us.
I think this is the biggest improvement in this proposal
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.
Right, but shouldn't the team be made aware of pending security issues to accommodate this?
At this moment, if we were to publish an update to a package, and we bumped the minor version there is no way of knowing that another security issue will be raised in the following days to which once again the package will have to have the minor bumped again and republished.
Will think on this some more overnight... 🤔
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.
I think creating this kind of patches for WordPress deserves its own section/doc.
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.
FYI: To be aware of, WordPress currently backports when possible all the way back to WP 3.7
This raises the possibility that Gutenberg may also require patching back to
5.0
For example, if a security issue is discovered later today and is proposed to be fixed in
5.1.1
it is conceivably possible that the packages be updated will have to be backported to WordPress versions5.0
and5.1
as such the Gutenberg brancheswp/5.0
,wp/5.1
, andwp/trunk
will all require updating at this time.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.
That's exactly why we need to leave "room" for bug fixes and security fixes in the package version number.
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.
Package version numbers cannot work like that as semantic versioning dictates that once
6.7.0
is published,6.6.9
cannot be published, nothing less than6.7.0
can be published.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.
What makes you say that? How do npm package maintainers publish security fixes for old major and minor versions without forcing people to upgrade?
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.
They don't, people are forced to upgrade to the version with the security fix.
For example, if a vulnerability is discovered in version
1.1.1
of a package, and that same package is now at version7.8.9
, the fix for the1.1.1
vulnerability must be released in either7.8.10
,7.9.0
, or8.0.0
. A1.1.2
or1.2.0
cannot be published.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.
I think you're wrong, here's an example https://reactjs.org/blog/2018/08/01/react-v-16-4-2.html
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.
This is where trying to align semantic versioning of packages with the WP release versioning clashes. The problem here is that a latest release of a package could result in new features to the WP version the security fix is for. This could be mitigated some by the use of the new feature flags functionality in GB but does complicate things.
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.
I just read through the semver specs and I'm not seeing this as something that is codified. Is this something that is documented as a rule somewhere?
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.
I don't think the spec has an opinion on this, but I've never read it that way. If so, something like PHP isn't following semver. Maintaining LTS for a specific minor release wouldn't be possible.
I always interpreted the spec as any individual version is just a comparison of the versions that are lower than it, regardless of release date.
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.
@nerrad @earnjam I was not inferring a SemVer spec, I was inferring it as a restriction of the npm platform, that said:
@youknowriad I. Did. Not. Know. That. 💥
(I've just confirmed with npm that this can be done, also advised not to publish the backported releases with the
latest
npm tag.)That will make things far simpler, though thinking about the "room" that we would leave behind, if version
1.1.1
of a package is already published, we bump the minor so that it becomes version1.2.0
, if we then have to patch version1.1.1
for a vulnerability, by publishing the fix in1.1.2
is that ok for semantic versioning? There is the possibility the "fix" breaks _backward compatibility which means the package should have been bumped by the minor version and not the patch version 🤔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.
Should
This can happen...
part be moved after the list of steps to make it easier to follow?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.
Maybe it can be reworded, but the reason it's here is to ensure that people are aware when to apply this flow before really applying it.
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.
This will be problematic for other projects using our packages, it is not semver friendly at all.
See also my above comment https://github.com/WordPress/gutenberg/pull/14136/files#r260688733
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.
What alternatives do we have?
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.
Continue to use semantic versioning:
If WordPress
5.1.1
is planned due to a security issue with the@wordpress/editor
package which is at the time of writing is currently version9.0.11
then the@wordpress/editor
package is patched, updated, and published as version9.1.0
.That version
9.1.0
version of@wordpress/editor
is then backported into both thewp/5.0
andwp/5.1
branches (andwp/trunk
)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.
The thing is
9.1.0
might already exist because thewp.5.2
might have already included it, so how do you go about patching the old versions without also adding features from the new versions?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.
For the past you cannot, the deprecations in the packages will have to support the previous versions of WordPress per the Gutenberg deprecation policy
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.
I'm not talking about breaking any compatibility here. I'm taking about:
WP 5.0.3
includes the editor package with9.0.11
WP 5.1
includes the editor package with version9.0.12
Trunk
includes the editor package with version9.1.0
(These are random numbers to illustrate)
A security issue is discovered and concerns all these packages, WordPress has to ship a security release for both
5.0
,5.1
and also fix the issue in trunk. How do we proceed?Contraints:
Which versions do you publish and include in the WordPress releases
5.0.4
,5.1.1
and trunk?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.
Based on the above #14136 (comment) knowing that we can backport the fixes going for the option of bumping the packages minor version number before release would allow us to avoid the above scenario...
As the WP 5.0 branch would have v9.0.x of the package, before releasing WP 5.1 the package minor is bumped to v9.1.x, this allows for multiple 9.0.x release to be backport puiblished for WP 5.0.x's, eg,
9.0.11
for WP5.0.3
,9.0.12
for WP5.0.4
,9.0.13
for WP5.0.5
etc etc, and for WP 5.1.x,9.1.1
,9.1.2
,9.1.3
and on are available, bump the minor again for WP 5.2 and9.2.x
range is available.For projects other than WordPress using our packages, a note on all the repos that a minor bump to all packages occurs when WP publishes a new version should document for the most part as to why we are slightly breaking semver on our packages, occasionly.
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.
In my example, this is not possible because
9.0.12
already existed and is used in5.1
and we don't have room to add fixes between9.0.11
and9.0.12
, that's why this is a special case using "metadata". I know that it's not perfect and that it's not "full semver" as npm will consider these versions as the same unless you explicitely target one but I don't think we have choices here because this scenario already happened, in this part, I'm trying to find a way to solve it when we face it.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.
Yes, I realise that, what I'm saying is we should do as you suggested above, and in the above scenario before WP 5.1 was shipped we would have bumped the editor package from the current
9.0.11
to9.1.0
, thus WP 5.1 would have shipped with9.1.0
and we'd have the remaining9.0.x
versions for as many WP 5.0.x releases required over the coming years, WP 5.0.55 would use9.0.66
😏