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

Introduce staging branch for stable release streams #6306

Closed
MylesBorins opened this issue Apr 20, 2016 · 32 comments
Closed

Introduce staging branch for stable release streams #6306

MylesBorins opened this issue Apr 20, 2016 · 32 comments
Assignees
Labels
meta Issues and PRs related to the general management of the project.

Comments

@MylesBorins
Copy link
Contributor

Having done quite a bit of work with v4.x and v4.x-staging I can first hand claim that having the staging branch has made life much easier when it comes to release time.

I'd like to propose using a staging branch workflow for our stable release streams. This could be started once v6 is officially released.

Benefits:

  • Land code when delta is closest to zero
  • Allows for nightly build off staging
  • Easier to automate testing prior to release
  • Catch problems early, not the day before a release

Problems:

  • v4.x-staging branch has generally not been treated as sacred. If the branch is going to be rebased against v4.x we will need to be ready to handle conflicts both in the branch and in other peoples remotes

/cc @nodejs/ctc

@MylesBorins MylesBorins added ctc-agenda meta Issues and PRs related to the general management of the project. labels Apr 20, 2016
@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

+1 to this. @nodejs/ctc ... I'd very much like to quickly get the CTC's thoughts on this on today's call.

@Fishrock123
Copy link
Contributor

Sorry - as I discussed in the CTC meeting, I'm not seeing where this has any benefits over the current master->stable situation. master is essentially the staging for the stable.

@MylesBorins
Copy link
Contributor Author

@Fishrock123 the biggest difference I would see between master and a staging branch would be the lack of semver major changes.

Doing the latest v5 release there were multiple commits that required a bit of extra work to get them to backport properly as they had diverged enough from the v5 stream.

In reality this is likely more an introduction of formalized process and an expectation that things are backported in a timely fashion. If there had not been so many conflicts it would have likely taken less than an hour to put together the proposal rather than half a day (spread across multiple days).

@MylesBorins
Copy link
Contributor Author

Another advantage would be in limiting the delta in the process between LTS and Stable. While it is true that LTS does require an extra formality (specifically that things live on v5.x for at least a week), should we really have that much of a different process between the two?

If this process is handled correctly it should allow us to distribute some of potentially time sucking work out of the release process, and also offers the ability for us to know that things are not going to land much sooner than the day before a release.

@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

The point is that master should not necessarily be the staging for stable. As we have seen with things like the recent regressions caused by the path performance updates, and as evidenced by the difficulty in separating out the semver-minor and patches that shouldn't land in v5.x, having a separate staging branch would likely make things significantly easier overall for folks doing the releases.

Also keep in mind that @thealphanerd has been doing a significant amount of the heavy lifting on many of these releases lately, having done most of the v4 releases and a handful of v5 releases. If he's arguing that having the staging branch for stable would make it easier, I don't think we should discount it.

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Apr 20, 2016

Another one to add to the list... we currently treat v5.x as sacred.

For this weeks release I had wanted to do a patch release, 6d9c0c9 is already sitting on v5.x though. We currently have no option aside from releasing a semver-minor release now... It is my own fault for having landed that commit... but the backport was originally asked for two weeks ago with the hope it would land last week.

If there was a staging branch we could ignore this commit and rebase.

@Trott
Copy link
Member

Trott commented Apr 20, 2016

I am in favor of this. master should correspond to the next major version, not necessarily the current major version.

@jasnell
Copy link
Member

jasnell commented Apr 20, 2016

I'll note here also that @bnoordhuis expressed a +1 on this also here: #6304 (comment)

@evanlucas
Copy link
Contributor

I'm +1 on this and agree with @Trott

@mhdawson
Copy link
Member

mhdawson commented Apr 21, 2016

I'm +1 and while I also like the idea on its own, one of my main reasons is that the people doing the work are saying it will make their life easier.

@Fishrock123
Copy link
Contributor

Fishrock123 commented Apr 24, 2016

the biggest difference I would see between master and a staging branch would be the lack of semver major changes.

Same thing from master to the staging branch though. As far as I can tell the only use for the v4.x-staging branch is to allow buffering for security releases.

Another advantage would be in limiting the delta in the process between LTS and Stable. While it is true that LTS does require an extra formality (specifically that things live on v5.x for at least a week), should we really have that much of a different process between the two?

The difference is, to my knowledge, only posed because the LTS has a guarantee that security releases will be absolutely minimal.

If this process is handled correctly it should allow us to distribute some of potentially time sucking work out of the release process, and also offers the ability for us to know that things are not going to land much sooner than the day before a release.

This is already possible. We (especially @rvagg) often land stuff onto v5.x outside of a direct release.


The point is that master should not necessarily be the staging for stable. As we have seen with things like the recent regressions caused by the path performance updates, and as evidenced by the difficulty in separating out the semver-minor and patches that shouldn't land in v5.x, having a separate staging branch would likely make things significantly easier overall for folks doing the releases.

I would like some elaboration on how it would actually make things easier. In my experience of cutting a good chunk of stable releases, the exact same things would happen but we'd end up maintaining two branches fro no particular reason.


Another one to add to the list... we currently treat v5.x as sacred.

Not quite like master, I think (know) a bunch of us have force-pushed it here and there. Mostly myself and Rod, who have done the majority of stable releases iirc.

For this weeks release I had wanted to do a patch release,

This isn't really the mentality Stable was built around though. It is designed to ship minors of they are, since we ensure with a high level of detail they are roughly as safe as patches.


In general, I'm not very interested in having additional burden on the Stable release process. Not at all in favor of this.

@rvagg rvagg removed the ctc-agenda label Apr 27, 2016
@jasonkarns
Copy link
Member

jasonkarns commented May 9, 2016

I'd like to point out that the current collaborator guide is slightly misleading as it implies a v6.x-staging branch should exist: https://github.com/nodejs/node/blob/2fee50658fe38bf6eeecb390d056df69f21acbbb/COLLABORATOR_GUIDE.md#how-are-lts-branches-managed

There are currently three LTS branches: v4.x, v0.10, and v0.12. Each of these is paired with a "staging" branch: v4.x-staging, v0.10-staging, and v0.12-staging.

As commits land in master, they are cherry-picked back to each staging branch as appropriate. If the commit applies only to the LTS branch, the PR must be opened against the staging branch. Commits are selectively pulled from the staging branch into the LTS branch only when a release is being prepared and may be pulled into the LTS branch in a different order than they were landed in staging.

Any collaborator may land commits into a staging branch, but only the release team should land commits into the LTS branch while preparing a new LTS release.

If a v6.x-staging branch is not created, the collaborator guide ought to be updated to accurately reflect the branching strategy.

@MylesBorins
Copy link
Contributor Author

@jasonkarns it is a bit confusing. At the moment v6 is not an LTS stream... it will become so in October, but until then it is managed via the Current release process

@jasonkarns
Copy link
Member

@thealphanerd thanks. Where is that process documented? I help maintain node-build and we have build definition files that track the various branches/versions (intentionally pulling from git, we have other definition files for the published releases). Hence my desire to fully understand the processes.

@Fishrock123
Copy link
Contributor

@jasonkarns I think you're probably looking for https://github.com/nodejs/LTS#lts-plan?

There may not be a definition of staging branches. Generally staging branches should not be relied on.

@jasonkarns
Copy link
Member

@Fishrock123 thanks, I'm familiar with that but seemed that it was mostly around the release plan and didn't contain many specifics around branch names and strategy. but I think I have what I need and will stop hijacking this thread now :)

@rvagg
Copy link
Member

rvagg commented May 11, 2016

I'm -0 on this, for roughly the same reasons as @Fishrock123 is expressing doubt:

  • we haven't been treating v5.x as sacred in practice, there has been plenty of force pushing
  • we've been trying to keep v5.x up to date already to get nightlies out from it, it doesn't really matter where nightlies come from but we've been using it as its own stable branch
  • we don't have much of a need for buffering commits and being able to reorder to get a security release out since for stable branches we just push out what's there including security updates—the only counterpoint to that is that there have been a couple of releases where I have preferred to keep to semver-patch but I can't say the reasons for that have been very defensible, simply preference-based.
  • the diff between master and stable should be fairly minimal over time and I wouldn't mind us trying to keep it that way, e.g. my comment @ [WIP] errors: add internal/errors module #6573 (comment)

I'm not seeing clearly the reasoning for the arguments above about "master being a staging for stable", perhaps someone can clarify what that's all about if you feel that's a strong point? v5.x has been a staging for stable as far as I'm concerned.

Having said that, if other folks in here who are cutting stable releases feel strongly enough about it (@evanlucas, @Trott), I'm not going to object. I feel it would add more process than value but it's manageable if it's genuinely believed to be helpful.

@Trott
Copy link
Member

Trott commented May 11, 2016

Having said that, if other folks in here who are cutting stable releases feel strongly enough about it (@evanlucas, @Trott), I'm not going to object.

For the record, I have not been cutting stable releases, so my opinion should not be given any undue weight. On the other hand, I believe all of @evanlucas, @thealphanerd, @jasnell, and @Fishrock123 have been involved in one form or another, so those are probably the ones to defer to.

@MylesBorins
Copy link
Contributor Author

I know that this has been stalled but I wanted to bring up what is currently going on with the release of v6.4.0

There are a handful of commits not landing nicely, this would have easily been avoided if we had maintained a staging branch with regular backports. The more I think about it the more I feel we should be using an identical release process for LTS / Current as far as staging and backporting is concerned.

Obviously there will always be pain, but we can at least deal with this pain early enoug hthat it will not block a release if we maintain a consistent staging branch and make regular backporting part of our release cycle for all release streams.

@Fishrock123
Copy link
Contributor

I'm willing to buy that it may be easier to know what branch to target, but all other reasons just end up being unnecessary.

The process is very unlikely to change at the pace of releases, and still oftentimes stuff will be merged into staging on the day of a release. I don't think that's a problem, just a reality to live with.

@Fishrock123
Copy link
Contributor

@thealphanerd Now that you can re-target PRs, is this even remotely necessary? It doesn't even matter which branch someone targets now.

We should have a backporting guide though I think.

@MylesBorins
Copy link
Contributor Author

@Fishrock123 that doesn't really affect the purpose of staging imho

vX.x branch is holy. it shouldn't ever be force pushed or rebased..
vX.x-staging is a development branch that can be rebase, force pushed, rev-parsed, or whatever needs to be done to get a release off the ground.

While this might not be as neccessary for Current as it is for LTS, but I see a ton of value in having a single process being used.

@jasnell
Copy link
Member

jasnell commented Aug 18, 2016

+1, the fact that prs can be retargeted does not really impact whether or not we have a staging branch.

@Fishrock123
Copy link
Contributor

vX.x branch is holy. it shouldn't ever be force pushed or rebased..

The same rule for force-pushing as master applies. ~10 minutes.

vX.x-staging is a development branch that can be rebase, force pushed, rev-parsed, or whatever needs to be done to get a release off the ground.

Will still end up always being done in the release PR, I don't see the benefit and would not like to spend more time & brainpower than already necessary to do releases. :/

@evanlucas
Copy link
Contributor

I'm actually +1 on this now. Up until now, I have always just cherry-picked to v6.x before a release. But since backport PRs are targeting the v6.x branch, how do we handle when stuff lands on v6.x that shouldn't go into the next release? I think we should have a v6.x-staging branch now.

@jasnell
Copy link
Member

jasnell commented Aug 25, 2016

Definitely still +1 to a staging branch. Having gone through and done releases with and without, I much prefer the ease of the staging branch.

@Fishrock123
Copy link
Contributor

how do we handle when stuff lands on v6.x that shouldn't go into the next release?

Could you please elaborate what this stuff is? If a minor lands they we do a minor release. There should not be a problem with that. It is what semver and tests are for.

@rvagg
Copy link
Member

rvagg commented Aug 31, 2016

how do we handle when stuff lands on v6.x that shouldn't go into the next release?

Note that we don't have a strict policy of only shipping security updates in Current branches, we just ship as usual and include security updates in the mix, unlike on LTS branches. We've left it up to releasers to decide on what to ship and occasionally releasers have chosen to release only a subset of what could be released (e.g. avoiding a semver-minor bump), but the reasons for doing that are not easily defendable, they come down mostly to preference (I say this as someone who has held back semver-minor commits, with not very strong justification). Also, if there are commits on there that you really don't want to ship, then just pull them out, reinsert and force push, I think we've given releasers that kind of ownership of Current branches when they are doing releases.

I'm neither for or against this but I have a preference for the status quo when the justification isn't very compelling.

@jasnell
Copy link
Member

jasnell commented Aug 31, 2016

At this point, the people who have been doing most of the release and backport work have very clearly and repeatedly indicated that having a staging branch for current would be ideal. Given that the cost is minimal and we can always go back to the current plan if it becomes too difficult to manage, I'd say that we should just go ahead with this.

@Trott
Copy link
Member

Trott commented Sep 6, 2016

CTC voted in favor of this. Removed ctc-agenda label, assigned to @thealphanerd.

@MylesBorins
Copy link
Contributor Author

I made a new branch v6.x-staging

new backport commits should be targeted at staging. I will go through the documentation and start updating things tomorrow

@MylesBorins
Copy link
Contributor Author

we do this yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
meta Issues and PRs related to the general management of the project.
Projects
None yet
Development

No branches or pull requests

8 participants