-
-
Notifications
You must be signed in to change notification settings - Fork 769
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
Adjust deploy scripts to archive old releases in a separate branch, move existing releases out of master #2426
Conversation
These archives made it difficult to find things in the GitHub interface, and take up a lot of space in a checked-out repo for something that is not useful to most people checking out the repository. The main purpose of these archives is to make old versions and documentation available on the Sinon website that is run out of this repo. This can be supported by using a separate branch for website releases, and to maintain the archives. Following this commit, the `npm version` scripts will be updated to automatically handle archiving the releases in the new releases branch and keeping it up to date with master. Also remove the directories we removed from .prettierignore, since they don't exist any more.
This updates the `postversion.sh` script to merge master into the new `releases` branch and then copy and commit the version archives there, instead of into master, so that the archives aren't cluttering up the master branch unnecessarily.
Previously we were updating the changelog and doing other auxiliary version bumps in the post-version script, which meant they weren't included in the tagged release commit, and required a followup commit. This moves the version-bumping changes into a new `version.sh` script, so that those changes show up in the tagged version commit.
We were manually running prettier on CHANGELOG.md during release but ignoring it in .prettierrc - this may have been a holdover from before Prettier had the proseWrap option, which would have wreacked havoc with the changelog, but the current version handles things just fine.
We shouldn't ever have conflicts here, so we can just auto-merge
$(<CHANGES.md) wasn't working on my machine, I think because it's a bash-ism, and my machine was running it under sh. I also took the chance to do something more useful than just `cat` by removing the double header that was previously on this page.
Also make version.sh a bash script like the others - this would have fixed the update-change-log-page script, in retrospect, but I think it's worth getting rid of the double header anyway.
Otherwise when we switch back to the source branch we'll have these files left dangling, which was what we were trying to avoid in the first place
Stringing commands together like this makes it so if the push doesn't work, we keep going, which we don't want!
This makes the script work on my machine, since I of course can't actually publish to sinon's npm package Also skip postbuild because chrome tests aren't working on my machine and `npm install` doesn't fix it
This is ready for review, but GitHub doesn't have a "ready for review but not ready to be merged" state. I marked this as a draft to be extra safe, because it shouldn't be merged as-is with the |
@mroderick Worth a peek! |
I just realized this PR is impossible to review on GitHub by default - but you can use the Commit Range feature to ignore the first commit that removes 2500+ files, and review the rest of the changes. This link should take you to the changes tab with the relevant commit range pre-selected, I've added this to the description as well! |
I am good with this! Works fine once I read your instructions 😄 |
Oy, seems it was not such a great idea to forget firing up the docs site on localhost before merging this. Not sure how I managed to forget that one, but this broke the docs site big time. This is my fault for not checking of course, but if I do not manage to figure this out quickly, it would be great with a hand. All the releases, that was moved out of /docs/, are not part of the deployed page. Ref #2432 |
fixed, there were just a few minor issues. one was merge conflict markers left after failing to realize I needed to not squash this, but keep history to get the whole |
Ah yeah, in retrospect I should have asked if there was some time where I could be on hand for the merge in case things went wonky, cause there were a lot of moving parts here - as-is it was something like 4am my time at merge-time and I was definitely not awake. Sorry for all the mess! I also should have thought about the squashing, I forgot that that y'all usually squash-merge - because yeah, the creation of the releases is some fairly delicate git-wrangling, and as you discovered the hard way you need at least the removal commit to exist on its own as a starting point for the releases branch. I'm reviewing the history now to make sure things should be in a good state moving forward, now that all the resulting conflicts and things are resolved, but immediately, it looks like the npm release didn't actually happen, because I didn't communicate clearly enough that the temporary commit should be removed before merging (along with the commit from the test release) The end result is that the |
Hah, good catch! |
Note: Due to so many files being removed this PR makes GitHub's change viewer unusable, but if you exclude the first commit using the "Commit Range" option, it should be much more reviewable! This link should take you to directly the "Files changed" tab with the first commit excluded:
https://github.com/sinonjs/sinon/pull/2426/files/fe65826171db69ed2986a1060db77944dbc98a6d..HEAD
Purpose (TL;DR)
A proposed solution to #2421. In this solution,
master
always has just the currentrelease-source
directory, and old versions are maintained in a new branch (provisionally namedreleases
), which is maintained alongsidemaster
and used to deploy the documentation site.Solution
In short, this revises the
version
scripts (including the pre/post) such that instead of copying therelease-source
into the archive directories in master, it switches to a newreleases
branch, mergesmaster
into that branch, and then does the copying and committing ofrelease-source
. It then switches back to the newly-releasedmaster
.This PR includes a one-time removal of all the existing release archives from the
master
branch, and will require a one-time creation of a parallel branch to contain the archives. Creating the new branch is pretty straightforward - it involves branching off ofmaster
at the first commit in this PR (the one that removes all the archived releases), and then in the new branch, reverting that commit to restore them. Once that is done, we can move forward mergingmaster
into the parallel branch as needed without re-removing everything.My fork has a full demonstration of an example release of the current repo's head, both with the current method, and with the proposed modifications under this PR. The relevant branches are as follows:
status-quo
is the product of runningnpm version v12.0.2-status-quo
on the currentmaster
commit, plus committing the uncommitted CHANGELOG.md changes that are left over (see "Other Fixes" below). This is (I think) whatmaster
would look like after a release currently.separate-releases-demo
is the product of runningnpm version v12.0.2-releases-demo
on theseparate-releases
branch*. This is whatmaster
would look like after a release under this proposal.releases
is the new parallel branch used in this example, and demonstrates how that branch would look after thev12.0.2-releases-demo
release and archiving.releases-base
tag is how thereleases
branch should be initialized before runningnpm version
with the new setup, and thus serves as the base forreleases
.Perhaps more useful than looking at the individual commits would be the network tab or your favorite branch viewer:
Also, I'm super not attached to the name for the
releases
branch, it could bearchive
ordoc-site
or whatever.The most recent commit in this PR is temporary and should not be merged - notably, it adds the
--dry-run
flag to avoid actually publishing a version. This was initially because I can't do that and it broke my builds, but you probably also don't want to run it for real under testing either! Assuming this gets merged, I'll remove that commit before we do.Other Fixes
Along the way, I also fixed a few other things that I noticed, particularly around the changelog - I see y'all recently updated how that is generated, and there were a couple rough edges left over, I think. Namely:
CHANGELOG.md
after user-edits as part ofnpm version
, but then not actually committing the prettier fixes - and they indeed seem to have never made it into the repo..prettierignore
, which seemed counter to linting it in theversion
pipeline. Removing them from the ignore didn't introduce massive changes or anything, so I did so.postversion
, the tagged release commit didn't have all the version updated everywhere, and theversion
script was adding a follow-up commit that did that tidying. This PR moves that logic into a newversion.sh
script, so that all the versioning updates are present in the tagged commit.How to verify
Note: These directions will not actually publish a package to NPM (Per above, I added the
--dry-run
flag in this PR) but it will actually create branches and tags in whatever repo you're pushing to!releases
branch, either by branching from this branch and reverting as detailed above, or simply setting it to thereleases-base
tag from my forknpm install
npm version v12.0.2-whatever-you-want
(again, this is a dry-run, I wouldn't blame you for double-checking the script first tho 😄)releases
branch, but notmaster
Other stuff
If you want to reset the state of your repo after testing, I used this
reset.sh
script to clean up the local and remote state during my own testing:In the
separate-releases-demo
branch you can't see my original additions toCHANGELOG.md
before linting, because it's now linted before committing. For reference, the most recent commit in thestatus-quo
branch shows what I added toCHANGELOG.md
, and what changes the linter made.Checklist for author
npm run lint
passes