-
Notifications
You must be signed in to change notification settings - Fork 20.6k
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
Release: migrate release process to release-it #5306
base: 3.x-stable
Are you sure you want to change the base?
Conversation
@@ -1,3 +1,5 @@ | |||
Authors ordered by first contribution. |
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 line was accidentally removed at some point. The main
branch still has it.
@@ -366,3 +337,6 @@ Simon Legner <Simon.Legner@gmail.com> | |||
Vladimir Sitnikov <sitnikov.vladimir@gmail.com> | |||
Anders Kaseorg <andersk@mit.edu> | |||
Alex <aleksandrosansan@gmail.com> | |||
Timo Tijhof <krinkle@fastmail.com> | |||
Gabriela Gutierrez <gabigutierrez@google.com> | |||
Dimitri Papadopoulos Orfanos <3234522+DimitriPapadopoulos@users.noreply.github.com> |
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 know the number of changes looks odd, but I've confirmed that grunt authors
has the same result. My guess is this has something to do with privacy rules being respected more accurately in git logs.
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.
No, and I actually made these changes long ago on purpose... This was to certify that contributions to Sizzle matter as much to jQuery as contributions to the main jQuery repo. I changed the file so that the entries are ordered by the first contribution to any of those two repositories. I'd like to preserve that.
Cf. https://github.com/jquery/jquery/pull/4395/files#diff-b897336dda0579c40f3623b68eb6652ded79e206f4cf1af7ceb24b683fd46ff1 & https://github.com/jquery/jquery/pull/5113/files#diff-b897336dda0579c40f3623b68eb6652ded79e206f4cf1af7ceb24b683fd46ff1
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. Let's incorporate that into the authors script then.
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.
It’d be interesting to see if I made any mistakes in this ordering. 😅
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've updated the authors script to clone the sizzle repo, combine the authors, sort by their first contribution, ensure they are unique by name, and then run cleanup so the sizzle repo doesn't stick around.
One case that I found interesting is Timo's entry. I noticed that his old email is still in the authors, whereas both emails were in there before. I think we only need the one; the email will automatically update when Timo updates the mailmap.
return changelog; | ||
} | ||
|
||
generate(); |
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.
Some of this was pulled directly from changelogplease, but grouping by component with ##
headers and using marked
to then convert to HTML is what's new. changelog.md
is also written to a file now and I've intentionally left that out of the .npmignore
so it's included in releases.
*Authors* - Checking and updating authors has been migrated to a custom script in the repo *Changelog* - changelogplease is no longer maintained - generate changelog in markdown for GitHub releases - generate changelog in HTML for blog posts *dist* - clone dist repo, copy files, and commit/push - commit tag with dist files on the 3.x-stable branch; remove dist files from main branch after release *cdn* - clone cdn repo, copy files, and commit/push - create versioned and unversioned copies in cdn/ - generate md5 sums and archives for Google and MSFT *GH actions* - add GitHub workflow for running releases in CI - releases can be triggered from GitHub; args can be added for pre releases - npm publish can now run with provenance Fixes jquery/jquery-release#114
@@ -2,12 +2,12 @@ | |||
"name": "jquery", | |||
"title": "jQuery", | |||
"description": "JavaScript library for DOM operations", | |||
"version": "3.7.1-pre", | |||
"version": "3.7.0", |
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 am not a fan of this change. With it, the version in package.json can no longer be universally trusted. Also, unless this was addressed somewhere else, Git builds will contain an incorrect version in the comment header and in the jquery
property.
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.
Judging from the discussion at release-it/release-it#946, it should be possible to keep our versioning strategy.
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 version in package.json can no longer be universally trusted
It never really could with -pre
either. It was often wrong about what version was coming next. -pre
is a convention we invented that I don't think has a place anymore. The concern was that people would mistake some state of the repo with the exact state used to release the last version. But, the practice of leaving the version and continuing to develop has become so common I don't think anyone actually makes that mistake anymore. It's part of the language of maintaining semantically versioned repos.
In fact, because what we do is so foreign, it has gotten increasingly confusing. Keep in mind that changing to -pre
obfuscates what the last version actually was. And the advantages of leaving the version alone have only been increasing.
If -pre
had caught on and become a standard, it might have been nice, but now we're the outliers.
Git builds will contain an incorrect version in the comment header and in the jquery property.
Again, I don't think -pre
version made sense either. As I explained in the issue description, git jQuery already has the SHA. All we'd need to do is remove the version from the header. I'm actually in the process of migrating our build off of grunt. I could tackle that change almost immediately. In fact, I was thinking all local builds could include the SHA by default, unless an explicit version is passed in.
Judging from the discussion at release-it/release-it#946, it should be possible to keep our versioning strategy.
I'm not sure that issue is related. It's talking about alpha and beta releases, which actually work better when the version in the package.json doesn't have a custom suffix (i.e. one that we never include in released versions). release-it has a much easier time determining the next version (and give you alpha and beta versions options) when the package.json version is left alone, which I think helps illustrate my point about the strangeness of what we do.
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.
OK, I see your points about package.json
.
I still feel a bit uneasy about the value of the jquery
property; it's true we add the commit hash to the Git version but AFAIK that doesn't happen for local builds from commits. Maybe we could tweak the build process to always read the current Git hash (instead of relying on the environment variable) and always appending it on non-tags?
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 was actually thinking that as well. I'm preparing a PR to migrate the build process off of grunt and I also like the idea of defaulting to adding the SHA (releases would specify an explicit version).
@timmywil are you targeting it at 4.0 or 3.x? I may not have time to do the full review before September 4 (it may happen but no promises). |
@mgol I mostly wanted to see how hard it was so I hadn't decided yet, but I think I should release 3.7.1 and we can keep churning on this. |
Summary
Authors
Changelog
dist
-pre
version. It's a practice we invented and have been doing a long time, but I'm not sure is actually necessary. It simplifies some release-it steps to not do it, which bases it's automatic version detection on the assumption that the version in package.json is the last version released. Besides, our-pre
version is often wrong anyway (for instance, when the next version is going to be a minor or major instead of a patch). If jquery-git.js is a concern, it does already include the SHA in the header and I think we could remove the version there completely.cdn
GH actions
Fixes jquery/jquery-release#114
Checklist