Skip to content
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 12.x #1896

Closed
paulmelnikow opened this issue Feb 11, 2020 · 39 comments
Closed

Release 12.x #1896

paulmelnikow opened this issue Feb 11, 2020 · 39 comments
Milestone

Comments

@paulmelnikow
Copy link
Member

I've opened a few PRs with breaking changes that I want to batch up for v12, though I suggest once that queue has been cleared we ship 12.x and resume working on improvements on master. In the past we used beta because we wanted to drop Node 6 and that would have severely held up development, though it would be a lot of overhead to maintain a beta and active branch and I don't think we have the capacity.

I like the idea of using short-lived beta branches this way: solely for batching up breaking changes; extra points if we've implemented the breaking changes it in a backward-compatible way with shims which can simply be removed.

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Feb 11, 2020

To minimize the number of PRs like #1897 which merge commits from master into beta, I suggest we also freeze the master branch except for security-related fixes bug fixes, and land any new PRs in the meantime againstbeta.

@paulmelnikow
Copy link
Member Author

I noticed semantic-release is not doing anything on beta.

@paulmelnikow
Copy link
Member Author

Hmm, it is running now, though it doesn't seem to know it's a breaking release. Did I mess up the semantic commit message?

@paulmelnikow
Copy link
Member Author

I opened #1898 to give this another try.

@paulmelnikow
Copy link
Member Author

That worked. 12.0.0-beta.1 is live: https://www.npmjs.com/package/nock/v/12.0.0-beta.1

@paulmelnikow
Copy link
Member Author

I minimally checked this version in a PR and it seems to work: paulmelnikow/icedfrisby-nock#154

I'll open a PR for the release.

@paulmelnikow
Copy link
Member Author

When #1899 is ready to merge, does it need to be done using a merge commit to correctly preserve the notes needed for semantic-release?

@paulmelnikow
Copy link
Member Author

It looks like when I merged #1894 into beta as #1897 it's taking it as a different commit, so there are merge conflicts. To fix that I think I need to merge master into beta with a merge commit.

It begs the question – will the same thing need to happen when it's merged back into master (i.e. a merge commit)?

@gr2m what would you suggest?

@paulmelnikow
Copy link
Member Author

I think what I did in #1897 is wrong, and that the first thing that's needed is a merge commit from master into beta to pull in the changes that have been made. After that I guess it needs to be a merge commit from beta back to master?

That's the only way I'm seeing, though I'm open to learning a different way this can be done.

@gr2m
Copy link
Member

gr2m commented Feb 12, 2020

Merge commits are ok in this case.

Also big +1 for short lived beta branches. I'd rather do 3 breaking releases instead of having a beta release phase that goes on for months

@paulmelnikow
Copy link
Member Author

Okay, I've turned on merge commits for the repo. I opened #1900 which we can merge with a merge commit and should get rid of the conflict, and then we can merge #1899 with a merge commit and 12.0 will get released.

@gr2m
Copy link
Member

gr2m commented Feb 12, 2020

I usually do the merges locally, so I don't need to change the settings, but that works too

@mastermatt
Copy link
Member

If possible, I'd vote for a "Rebase and Merge" on #1899 so the merge-in commits will be removed and the end master branch will be cleaner, while still maintaining the changes we want.

@paulmelnikow
Copy link
Member Author

At #1871 (comment) @gr2m mentioned that semantic-release is using notes on the commit. Will rebase and merge work with that, especially once we delete or move aside the beta branch? If that works, I'd prefer it too.

@gr2m
Copy link
Member

gr2m commented Feb 12, 2020

I think rebasing & merging into master should be okay. The problem is that once you rebase beta, semantic-release won't know what to do with it. But when we delete the beta branch anyway, we should be okay.

I'd suggest to do the rebase and merge beta -> master locally, without pushing the rebased beta back to GitHub. After that, delete the beta branch until the next time we need it

@paulmelnikow
Copy link
Member Author

Okay. I think the rebase + merge button will do that. It'll pull in the merge commits. Does that sound good?

Alternatively I can take your suggestion to do a manual rebase locally. If I do that I would want to check it in GitHub before merging it to master. Besides checking that I haven't made any merge mistakes, I think it's better that anyone can see what I'm proposing to do. I can push it to something named other than beta, so semantic-release will leave it alone until it's merged.

@gr2m
Copy link
Member

gr2m commented Feb 12, 2020

yeah you can do that. semantic-release will fail because it will try to re-create the -beta.1 tag, but you can ignore that, no harm should come from it

@paulmelnikow
Copy link
Member Author

yeah you can do that.

Do that = push the rebase + merge button?

@gr2m
Copy link
Member

gr2m commented Feb 12, 2020

Yes. Just be 100%, I understand you want to

  1. push the rebased beta branch with git push --force
  2. merge beta into master using the rebase + merge button

@paulmelnikow
Copy link
Member Author

No, the first option was:

  1. Merge beta into master as it stands now, by pressing rebase + merge on Release beta branch as 12.x #1899

The other one is:

  1. Create a new branch e.g. release-12x from master, and add to it all the new commits from beta in straight line.
  2. Push it to GitHub and confirm it looks the same as beta.
  3. Open a pull request against master. and merge using rebase + merge.

@mastermatt
Copy link
Member

I just ran a rebase of beta on master on my local. It was clean with no errors. I think you should just go with the first option. It's the easiest and should do the same job in the end.

You can see the result here master...mastermatt:release-12x

@paulmelnikow
Copy link
Member Author

Huh, there are no merge commits. It looks cleaner than what's in #1899.

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Feb 13, 2020

Why don't we use rebase + merge on that? #1901

@mastermatt
Copy link
Member

Using the Rebase and Merge option on 1899 would have the same effect. Merge commits and noop commits are removed as part of the Rebase. 1899 and 1901 are effectively identical now.

@paulmelnikow
Copy link
Member Author

Okay, well, let’s merge #1899 then. It’s always great to understand this stuff better!

@paulmelnikow
Copy link
Member Author

Welp, the commit history looks great, though it was released as 11.9.0 instead of 12.0.0 and I've no idea why. The same thing happened initially on the beta branch when I merged 6c504c3. Maybe it needs to be BREAKING CHANGE: ... and not BREAKING CHANGE? When I pushed 31bf521 it righted itself though that didn't seem to make a difference here. I guess it's what @mastermatt said about no-op commits being removed as part of rebase.

I guess we should publish a 11.9.1 which is the same as 11.8.2 (manually?) and then add a commit with the correct change entry to get the 12.x release out?

Argh…

@gr2m
Copy link
Member

gr2m commented Feb 13, 2020

aybe it needs to be BREAKING CHANGE: ... and not BREAKING CHANGE

Yes :( The : is significant.

First thing: set @latest to the 11.8.2 on npm

npm dist-tag add nock@11.8.2 latest

I'd release a fix commit that reverts the changes, then deprecate 11.9.0. Then do the release for 12 proper.

I can help with that on Friday. But I would revert the breaking change as soon as possible. Do you need help with that?

@paulmelnikow
Copy link
Member Author

Can you add me on npm?

@gr2m
Copy link
Member

gr2m commented Feb 13, 2020

done

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Feb 13, 2020

I'd release a fix commit that reverts the changes, then deprecate 11.9.0. Then do the release for 12 proper.

I can help with that on Friday. But I would revert the breaking change as soon as possible. Do you need help with that?

This makes sense, though I also think it would be nice not to have the two revert commits in the main branch. The history is better as it is, it's only the release that's wrong. Could we do what you're suggesting as a maintenance release on an 11.x branch?

Added: Either way the changelog is going to need to be fixed manually.

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Feb 13, 2020

So far it's not showing yet on the npm website. I don't see the Admin tab and I get a 403 when I update the dist-tag. I'll keep trying.

Added: I see it here: https://www.npmjs.com/settings/paulmelnikow/packages. Clearly it's gonna take a minute to propagate.

@gr2m
Copy link
Member

gr2m commented Feb 13, 2020

just fyi I run npm dist-tag add nock@11.8.2 latest already

@mastermatt
Copy link
Member

Where are we with this?
I see 11.8.2 is tagged as latest, but 11.9.0 is still on npmjs.org and there is no 12.0.0.

Am I right that the hash of 11.9.0 should be retagged and published as 12.0.0 and 11.9.0 should be deleted?

Don't we only have a couple of days to delete accident versions on npm?

@gr2m
Copy link
Member

gr2m commented Feb 16, 2020

Let me push a fix patch version 11.9.1 and then look into beta, ok?

@gr2m
Copy link
Member

gr2m commented Feb 16, 2020

Once #1905 is merged, I'll redo the beta branch and fix the commit messages to actually trigger a breaking release, sounds good?

@paulmelnikow
Copy link
Member Author

If that's not a lot of work, then sure!

@paulmelnikow
Copy link
Member Author

paulmelnikow commented Feb 16, 2020

Actually, I think we should just make a branch that's to be merged into master (using the rebase + merge button). I don't think there's any point running it through beta again.

@gr2m
Copy link
Member

gr2m commented Feb 16, 2020

Agree

@paulmelnikow
Copy link
Member Author

Thanks @gr2m!

I've tweaked the 12.0.0 release notes and I think we're back on track.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants