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

Major version bump #1061

Closed
chrisdickinson opened this issue Mar 4, 2015 · 39 comments
Closed

Major version bump #1061

chrisdickinson opened this issue Mar 4, 2015 · 39 comments
Labels
discuss Issues opened for discussions and feedbacks. meta Issues and PRs related to the general management of the project.
Milestone

Comments

@chrisdickinson
Copy link
Contributor

By now we've gathered a sizeable number of PRs liable to require a major version bump. I've listed them below:

subsystem issue title issue links
process,lib fix process.send() sync i/o regression #760 #774
url significantly improve the performance of the url module #933
url re-add path in url.format #893
util fixed behavior of isObject() #822
os remove trailing slash from os.tmpdir() #747
freelist Remove Ryan's Freelist feature #569
deps Moving to V8 4.{1,2}, reverting floating V8 patch* #1026 #952

*: I might have noted this one incorrectly! Please let me know if this is in error.

I echo @domenic's sentiment here: I'd like to keep the number of breaking changes per major bump few in number, so this seems like a fairly good set of issues.


Our current setup for major bumps:

  • Cut a new branch – v2.x.
  • Bump the appropriate node_version.h version. Commit as "Starting work on v2.x".
  • Merge the PRs constituting the major bump to v2.x.
  • Switch the Github primary branch to v2.x.
  • We may have to update Jenkins? @rvagg would you know more about this?

As a side effect of this:

  • All existing PRs will be targeting the wrong branch (v1.x), including the PRs we would merge into the new v2.0.0 release.
  • We also will have to decide on a course of action for supporting v1.x – that is, do we go the joyent/node approach and land PRs in the "oldest applicable" branch and forward port, or land in the current development version and backport? (I am in favor of the latter, but want to present both options.)
  • Finally, the path we choose going forward with branch names & landing patches will influence our options for LTS support: if we keep vN.x branches, we are able to backport semver-minor changes to old versions, if we keep vN.N.x branches, we are only able to backport semver-patch changes.

A proposal:

Short-term: Do not cut major branches until we bump. Skip cutting the v1.x this one time, since it already exists. Develop current major version off of the master branch. Only cut vN.x branches at bump time. The downside is that all PRs will be mistargeted, but I don't think there's any avoiding that now. Land all changes in current development version, backporting them as necessary to older major lines. We keep vN.x, but commit to only backporting minor level patches sparingly; as an insurance policy for situations that warrant bringing a more secure API back to an old version. Generally only patches will be backported.

Long-term: I'd like to see the node_version.h fields templatized, so that the build system can grow a "release" job that can stamp the numbers in without having to affect git history. Currently that information is duplicated from the repository tags into the working tree, and I think it would ultimately be cleaner/easier to tool if it were purely repository metadata.

Thoughts on this, @iojs/tc, @iojs/build?

@chrisdickinson chrisdickinson added this to the 2.0.0 milestone Mar 4, 2015
@tellnes
Copy link
Contributor

tellnes commented Mar 4, 2015

The only way to let all existing prs target the correct branch is to continue develop on the v1.x branch. I don't think we want to do that. So +1 on moving development to the master branch.

Also +1 on creating vN.x branches and cutting LTS releases from that. If we in the future want to cut a LTS release for a older minor version where there exists a newer minor version in that major line, we can just create a vN.N.x branch for that particular case.

@mikeal
Copy link
Contributor

mikeal commented Mar 5, 2015

+1 on master being current and cutting .x branches after major releases.

@trevnorris
Copy link
Contributor

+1 letting master be the latest major version, and only branch when it's going into maintenance.

@indutny
Copy link
Member

indutny commented Mar 5, 2015

+1 for master.

@Fishrock123
Copy link
Contributor

If we are removing freelist, can't we also remove sys?

@retrohacker
Copy link

👍 for master

In my head, LTS means a feature freeze with a focus on maintenance to reduce the chance of merging changes that introduce new bugs. Are there any huge risks in only allowing symver-patch changes in?

@brendanashworth
Copy link
Contributor

+1 for master and splitting branches at bump time. @Fishrock123 I still think a lot of modules depend on it, but one of the issues with the sys deprecation is that most of the functions it would then call is also deprecated in util, too, so it mostly makes sense to remove them too - and then we realize we just broke a bunch of code because we removed "easily-maintainable deprecation notices", as someone put it.

edit: when we split new branches, can we add a change to the README saying that it is a legacy branch? that way less people get confused.

@qfox
Copy link

qfox commented Mar 6, 2015

👍 for master.

@Fishrock123 Fishrock123 added the discuss Issues opened for discussions and feedbacks. label Mar 6, 2015
@benjamingr
Copy link
Member

👍 for master, it sounds reasonable.

@silverwind
Copy link
Contributor

Adding the possible deprecation of the sync methods setServers and getServers in favor of an option on the async resolve here. Not sure yet how feasible it is to have it work async, so it's a bit of a unclear situation.

Related: #1071 and #894 (current crash bug)

@sam-github
Copy link
Contributor

Why merge major PRs at all? Leave them tagged semver-major and accepted (or something), and don't merge them until TC decides that its time to bump. Then branch a maintenance version of the last major, bump the version on master, merge all the pending accepted major PRs, and keep on going forward.

It's not at all clear to me why io.js is currently on v1.x branch and not master. I assume it is some accident of history, probably that master was supposed to be like something in joyent/node, but having an unused master seems odd at this point.

@chrisdickinson IMO, #956 is also semver-major.

@silverwind
Copy link
Contributor

Please disregard my previous comment about the dns.setServers deprecation. After further consideration, I think it's not worth the trouble to deprecate an actively used API for a minor issue that could probably solved in userspace (once #894 is done).

@indutny
Copy link
Member

indutny commented Mar 11, 2015

@sam-github leaving them in PRs is a free ticket to hell. Maintaining external PRs will certainly be a nightmare.

@qfox
Copy link

qfox commented Mar 11, 2015

@indutny With zombies and roof 🏠 in da 🔥 ? 🌋

@indutny
Copy link
Member

indutny commented Mar 11, 2015

@zxqfox not minecraft's nether, real hell.

@sam-github
Copy link
Contributor

@indutny merging major PRs intermixed with non-majors continuously onto master sounds like a ticket to hell, too! You'll have to cherry-pick all the non-majors out of master onto the current major by hand. Fun. Or bump major every month.

So, yeah, maintenance is easier for you to do because the code was merged to io.js... but if you leave them on the PRs you can ping the originator and say: "We're ready to bump the major and merge this, but its your responsibility to rebase onto the current master before we do. You have 2 weeks or you miss the merge window.". That may be hellish for the proposer of a PR that is a major, but it doesn't seem to me to be your personal hell.

Anyhow, I'm not clear after having read the comment thread what the current proposal is.

@mikeal
Copy link
Contributor

mikeal commented Mar 12, 2015

IMO we should just keep a canary branch where we merge any change that would bump the major. The concerns about this diverging too far from master are true but that's just another incentive for us to release these more often and avoid bundling a huge number of major changes in to a single release.

@chrisdickinson
Copy link
Contributor Author

@indutny merging major PRs intermixed with non-majors continuously onto master sounds like a ticket to hell, too! You'll have to cherry-pick all the non-majors out of master onto the current major by hand. Fun. Or bump major every month.

I think you may misunderstand the proposal. After this point, master is the current major. When we introduce another breaking change after v2, we will cut the v2.x branch out of master before merging the breaking change. Master will then represent v3.x. This keeps all of the PRs relevant.

An illustration:


                    X master
                    |
          X master  o v2.x
          |         |
  o v1.x  o v1.x    o v1.x
  |       |         |
  o       o         o
  |       |         |
  o       o         o
  (i)     (ii)      (iii)

  (i):   current status. "o" is any minor/patch commit 
  (ii):  near future. "X" represents the "working on v2.0.0" 
         commit + the major semver commits.
  (iii): future. "X" represents the "working on v3.0.0" commit. 
         Note that at this point the v2.x branch is cut.

              ∆ master (backported commit)
              |
  X master    X  ∆ v2.x
  |           | /
  o v2.x      o  ∆ v1.x
  |           | /
  o v1.x      o
  |           |
  o           o
  |           |
  o           o
  (iv)        (v)

  (iv): post-v2.x world, same as (iii) above
  (v):  we want to merge a minor/patch level commit and 
        backport it. we:
        1. merge it to master, creating commit ∆.
        2. checkout v2.x. cherry pick ∆ onto v2.x.
        3. checkout v1.x. cherry pick ∆ onto v1.x.

IMO we should just keep a canary branch where we merge any change that would bump the major.

I'm not sure the added ability to manually sequence minor/patch commits with major commits is worth the added complexity of a canary branch. With the master + v(N-1).x approach, current development is always on the latest version, and minor/patch commits can be cherry-picked into older release lines.

@ghost
Copy link

ghost commented Mar 13, 2015

I agree with @mikeal on this one.

@mikeal
Copy link
Contributor

mikeal commented Mar 14, 2015

I'm not sure the added ability to manually sequence minor/patch commits with major commits is worth the added complexity of a canary branch.

I don't think that we can expect users to be downloading and regularly testing more than one "future nightly" build. So either we aren't going to test major patches or we're going to also merge them in to this branch. However, you could have a "major" branch and a "next-v8" branch that get auto-merged for the canary nightly build.

@chrisdickinson
Copy link
Contributor Author

I don't think that we can expect users to be downloading and regularly testing more than one "future nightly" build. So either we aren't going to test major patches or we're going to also merge them in to this branch.

I think the canary/stable release problem is better solved by labeling individual semver releases as "stable" or "canary" vs. baking that into VCS and existing release system – npm's dist-tag scheme is what I'm envisioning, here.

@mikeal
Copy link
Contributor

mikeal commented Mar 16, 2015

I think the canary/stable release problem is better solved by labeling individual semver releases as "stable" or "canary" vs. baking that into VCS and existing release system – npm's dist-tag scheme is what I'm envisioning, here.

Here is where I don't agree at all.

First of all, if something is unstable we shouldn't give it a semver. Once it has a semver we can't remove anything in it without bumping the major. Also, as we release more we can expect people to actually use and rely on the engines field more often and we don't want them relying on unstable versions where we might remove something.

In order to keep steady releases we need to manage what lands in branches rather than managing what is "released." If the stability policy is about what lands in different types or releases we fall in to an endless argument about when a line is "stable."

What exactly is wrong with producing nightlies off a branch that we consider unstable and are testing?

We're getting way too concerned about how far this branch might diverge. If it's diverging a lot we should merge it, that's the whole point of doing regular releases, to reduce the number of changes in any single release.

@chrisdickinson
Copy link
Contributor Author

What exactly is wrong with producing nightlies off a branch that we consider unstable and are testing?

Here is my line of thought. Hopefully this makes the reasoning behind my concerns a bit more clear:

Assume that we go the route of a canary branch. What goes into the canary branch? All incoming commits? Only major-level commits? If we go with the former, master ends up representing a series of merge commits at the various points where releases were cut. Since that information is already neatly represented by git tags (for specific releases) and git branches (for ongoing work on a release line), let's go with "only semver-major-level commits go into canary."

To cut a release of canary that bears resemblance to a full release, master will have to be merged into the canary branch before the release. This is unlikely to be able to be done automatically, since semver-major level commits tend to be sweeping changes and merge conflicts are likely to occur. Because this process is manual, canary releases are no longer nightlies. They're more akin to Node's v0.11.x line of releases. We have experience with that system and its downsides:

  1. Because master must be manually merged, releases of this branch are sporadic rather than regular.
  2. While resolving merge conflicts, the person doing the merge and the people who authored the conflicting code are not one and the same, complicating merges.
  3. While we can put rules around the merge process – who should merge up and when, how often releases should be cut, etc. – these are rules for humans that the codebase itself doesn't enforce with structure. Humans have a tendency to work their way around, or otherwise misimplement, these sorts of rules.
  4. Even when those social rules are well-enforced within a given group, they are still hard to convey across "generations" of maintainers. Simple, repeatable rules are easier to convey and internalize; "rule of thumb" rules are more prone to distortion over time.

Using a canary branch in this fashion feels like a strategy we've already tried; one that was not worth the complexity it incurred.


First of all, if something is unstable we shouldn't give it a semver.

We don't (or can't, really) know if something is stable until people use it and prove that it is stable for their purposes. There are ways we can get a better estimate of whether something is stable – testing against npm ecosystem, etc – but ultimately bugs are an unknown unknown. Unless we start tagging release candidates for individual semver X.Y.Z releases (which seems like a huge amount of overhead), it's inaccurate to say that we only cut stable semver releases.

Once it has a semver we can't remove anything in it without bumping the major.

Yes, we can't remove any public API without bumping the major. However, I don't think this will be a problem in practice. If we release something that has a bug in canary, we can iterate on fixing that with patch releases on the canary line. The current stable (whatever semver version that is) would be unaffected, and most users would use that. If the current stable has a bug, we can also iterate on the canary line until we're certain we've fixed it.

We can also mitigate the risk of having to bump major due to public API changes by guarding large new features with command line flags, or otherwise soft-landing new functionality.

If we're worried about stable jumping from v1.6.0 to v4.0.0 because of the changes we're making, we also have the pressure release valve of the -rcN suffix. I'd prefer not to go that direction, but it's an option.

If the stability policy is about what lands in different types or releases we fall in to an endless argument about when a line is "stable."

We should crib the working release proposal model we're using now in order to propose stable releases. Any interested party can propose that a given canary release be promoted to stable. This centers discussion around a single release, where the merits (or demerits) of that particular vintage of io.js can be debated.

@mikeal
Copy link
Contributor

mikeal commented Mar 17, 2015

We should crib the working release proposal model we're using now in order to propose stable releases. Any interested party can propose that a given canary release be promoted to stable. This centers discussion around a single release, where the merits (or demerits) of that particular vintage of io.js can be debated.

I'd like to see a proposal for this. My experience to date talking with the community has been almost universally positive about the speed and cadence of releases since we started node.js.

@sam-github
Copy link
Contributor

@chrisdickinson What you mapped out above is exactly what I thought you meant. And I support it, btw.... from a how-should-it-look-in-git perspective.

Except that your proposal didn't take a stance on two issues that I think are critical:

  1. with what timing will major PRs be merged?
  2. what is criteria to determine whether non-major commits on master are cherry-picked to previous major releases?

These issues are closely related.

On the first point, you list 7 major changes at the top of this issue. io.js has been releasing for about 10 weeks. If those changes were evenly distributed in time, and merged into master as soon as they got a LGTM, io.js would be somewhere close to v7.x right now (probably a bit less, because releases have been every week or two, so some would get batched together).

The almost universally positive feedback @mikeal (and I) am hearing about io.js release cadence might not persist if io.js is at v30.x by next December.

What is your suggestion for this? Merge major PRs as they come, bump the major on master, and go for it? Or do you have some other proposal for slowing down the rate of release of MAJOR versions of io.js?

My suggestion is that major PRs get delayed to some predictable time-frame, like 3 months, or the v8 release cycle, something..., so that we will only be at io.js v4.x or so by next December. Whether that delay means rebasing them onto a canary-like branch, whether a -next tgz is built so people can try the bleeding edge if they want, or whether they are just tagged in github and left open, or some other proposal, is just an (important) detail.

Note that this is how projects like express work: express bumped from 3 to 4, but only after enough 4.x-worthy changes had accumulated, not continuously with each PR that worked towards 4.x

If io.js bumps major every month, I predict you are going to get a lot of pressure to cherry-pick _all_ non-major changes back to the last major release. And if majors bump often, to the last 3 or 4 major releases. That sounds like a lot of work.

@sam-github
Copy link
Contributor

Oh, and @indutny, you said "leaving them in PRs is a free ticket to hell." .. this is exactly what is being done right now.... all the major-changing PRs at the top of this issue have been benched... they are awaiting agreement that its OK to bump the major. Does it feel like hell already? It doesn't seem so bad to me, but perhaps I'm not seeing the pain.

The current state is pretty much what I propose... except that now there isn't any predictability as to when they will get merged. The authors don't know. Could be next week. Could be September. Could be never.

This is a bit like 0.12 release cycle, when all of our changes just sat in joyent's master for up to 2 years... with nobody knowing when they'd see the light of day. I propose that the TC establish a regular non-random cadence for majors.

@qfox
Copy link

qfox commented Mar 17, 2015

@sam-github Just imagine how to merge a PR that was developed by somebody a 2-3 months (or 2 years) ago. It's a free ticket for sure, but still not a ride. It's a risk that can be reduced by merging these PRs to some branch.

@sam-github
Copy link
Contributor

@zxqfox nothing is perfect. And I'm not opposed to merging major PRs to a next-major branch, or something.

But I don't agree that merge conflicts on PRs must be the responsibility of io.js collabs.

Its normally the responsibility of the proposer of the PR to keep them in a state they can merge clean.

Especially if PR proposers knew that every 3 months major PRs would get pulled in. And they knew when that would be. And they clearly know their PR is semver-major... well, its going to be their responsibility to rebase their PR so it can merge clean.

@chrisdickinson
Copy link
Contributor Author

@sam-github:

What is your suggestion for this? Merge major PRs as they come, bump the major on master, and go for it? Or do you have some other proposal for slowing down the rate of release of MAJOR versions of io.js?

I'm in the "integers are cheap" camp: if we're going to do semver, this is what semver entails. What we're doing right now, and what most proposals seem to suggest doing, is cooking the books a bit. We do so at present by letting major changes idle in PRs. If we switch to a canary branch, we cook the books by merging major changes to a separate major branch. Either way, the end result is that we defer the work of fixing merge conflicts until some later time. I'd rather not get to the point where merges happen >1 month after the change. Fixing merge conflicts is some of the hardest work to get right – and it only gets more difficult the longer it's put off.

That said, this bears more thought. What we're trying to get to is a world where we have the upsides of the even/odd release model, without its downsides. The upside to that model was the natural "canary" set of releases – the odd version line – for any stable release, where users would stick to the stable release.

@rvagg
Copy link
Member

rvagg commented Mar 17, 2015

I'm +1 on anything that retains, or introduces friction on bumping major. Remember the guiding philosophy of Node that remains one of the main reasons we are all still here: small core, vibrant userland.

Core shouldn't be innovating faster than userland, we should be paving cowpaths. If we free ourselves up to move faster on breaking changes and major new features then we'll end up in the land of suck that we get from browser vendors (think fetch() and even the new streams API) and to some degree TC39 which is still somewhat disconnected from those on the ground.

Our job is not to make the awesome happen, it's to facilitate awesome happening outside of core--likely by people other than ourselves! Think about that next time someone proposes a major new feature, or you find yourself tempted to introduce something new.

@chrisdickinson
Copy link
Contributor Author

@rvagg:

I'm +1 on anything that retains, or introduces friction on bumping major. Remember the guiding philosophy of Node that remains one of the main reasons we are all still here: small core, vibrant userland.

I agree 100% with the sentiment that core should remain small and focused, and that the primary value of io.js is in the userland ecosystem.

However, the value of the major version number does not affect the size of core. Major version bumps are likely to smooth a rough edge in an API, fix inconsistent behavior, or upgrade transitive dependencies (like V8.) These are things that may cause a user to have to change their code, but it is by no means guaranteed – io.js is essentially a grab-bag module after all.

Minor version bumps, on the other hand, represent strictly new code. If we want to keep a small core, we should be introducing friction for minor version bumps. Major bumps are "platform and codebase health" changes, not a vetting ground for net new features.

@mikeal
Copy link
Contributor

mikeal commented Mar 18, 2015

I agree with @chrisdickinson on this one. I'm all for having more major version bumps if the list of changes looks like the one we have now (all small improvements but because they are minor changes in behavior we have to bump major).

I don't think that causing friction in doing a major release does anything other than increase the breakage they cause in userland and curb their adoption by the ecosystem. We have plenty of evidence this is the case by looking back on prior major releases of node.js, the longer the release took the longer it took for it to be adopted and the more things it broke.

@mikeal
Copy link
Contributor

mikeal commented Mar 18, 2015

That said, I think that bumping major any more often than Chrome does is going to be confusing and kinda scary for users :)

@jamescostian
Copy link

@chrisdickinson pointed out that if there is a canary branch, then non-semver-major commits must be merged onto it, and that means manual work and no nightly releases for canary

@indutny pointed out that leaving pull semver-major pull requests to get stale and require manual merging is not exactly optimal either.

And everyone agrees that merging every single major pull request as if it were any other pull request => the X in X.Y.Z will explode

There's one common thread here - all of the above ideas imply that when there is a semver-major change, it should be described in code and submitted as a pull request. But what if semver-major changes were simply described in issues with a semver-major tag? Once enough issues with that tag have accumulated, you comment on all of them saying that pull requests are being accepted. At that point, everyone can make pull requests (aimed at master) and they can be accepted just like all other pull requests, and once all of the semver-major issues have pull requests which have been merged into master, then you cut the release and increment the X in X.Y.Z

This is the only way to avoid all manual merging IMO. And I'm pretty sure everyone agrees that relying on manual merging (which is susceptible to human error) is a very bad thing.

@chrisdickinson
Copy link
Contributor Author

For now @bnoordhuis has volunteered (thanks!) to cut a "next" branch and start merging majors into it, while merging minors and patches from v1.x to that branch.

@mikeal
Copy link
Contributor

mikeal commented Mar 19, 2015

when do we make the switch to master?

@jbergstroem
Copy link
Member

I'm pretty keen on moving way from v1.x as well. Makes it so much easier to cut stable releases.

@chrisdickinson
Copy link
Contributor Author

The plan is to switch to master after the first v2.0.0 release.

@Fishrock123 Fishrock123 added the meta Issues and PRs related to the general management of the project. label Mar 24, 2015
@Fishrock123
Copy link
Contributor

Since we've got the ball rolling on this, I think we can close.

Discussion for 2.0.0 can continue in #1532

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

No branches or pull requests