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

RFC: reduce use of minor release branches #8189

Closed
sminnee opened this issue Jun 18, 2018 · 31 comments
Closed

RFC: reduce use of minor release branches #8189

sminnee opened this issue Jun 18, 2018 · 31 comments

Comments

@sminnee
Copy link
Member

sminnee commented Jun 18, 2018

Right now we run minor release branches - 4.0, 4.1, 4.2, etc - and in as a merge rule, we:

  • Accept any bugfix change that it made to these branches, and/or
  • Encourage the submission of bugfixes against these branches rather than 4

I don't think this approach is in the best interests of the project and I'd like to suggest that we change. I think that these branches should be left in the system for as long as we're providing security fixes for these versions, but such branches should only receive:

  • Critical bugfixes
  • Security fixes

I think that the current approach has been chosen because of 2 assumptions, that are no longer true:

  • It's relatively cheap to maintain multiple parallel releases
  • It's relatively expensive to perform a minor release upgrade

Is it relatively cheap to maintain multiple parallel releases?

  • As we break our codebase into more packages, and the merge-forward efforts scale up, even with automation tools.
  • Each of our modules ends up with complex travis testing set-ups, testing against different versions of recipe-core.
  • Thanks to our government contact, SilverStripe receives quarterly independent security audits on changes made the code. Each release we add to this process scales costs up and we it's not really viable to pass multiple versions of 4.x through to these. Producing a "stable" release line that doesn't receive as much security auditing seems suboptimal.
  • When we need to release a critical fix such as a security fix on 4.1, it risks being cluttered with a number of less critical changes. In this situation genuine "hotfixing" requires creating forks of core modules (gross), or producing releases such as 4.1.1.1 (unnecessarily complex releases)

As our suite of packages grow and the level of testing we put into it increases, there more costs for maintaining multiple parallel releases. It would be better if the upgrade path was more like a single line than a tree.

Is it relatively expensive to perform a minor release upgrade?

In the bad old days, performing a minor release in SilverStripe was a risky process involving a lot of mandatory amendment of code to fix now-broken APIs. Since 3.3 we've worked a lot harder at making semver-compliant minor releases, and although we're not perfect, we're a lot better.

There will always be people who want to take as conservative a position as possible, but providing them only with critical bugfixes and security fixes should be sufficient, if they have pinned to ~x.y.z.

For anyone who wants to receive ongoing bugfixes, it seems fine to recommend that they instead keep pace with minor releases ^x.y.z. This is very different proposition than it was a few years ago.

Shouldn't we just leave it to the discretion of the core committer involved?

This is kind of part of the status quo solution. The issue is that it creates unpredictable outcomes, and it makes it harder for new core committers to know what approach they should follow. It's not as bad as everybody picking which side of the road they should drive on, or whether to use tabs or spaces, but I think it is much better if we have a clear guideline that everyone follows.

PROPOSAL: Update our merging guides to say that branches of the form X.Y should only receive critical bugfixes and security fixes, and any other documentation or bugfix changes should go be pointed to X instead.

👍 / 👎 please?

@tractorcow
Copy link
Contributor

I would add to that "creating a new minor branch (e.g. 4.2) auto-merges, then deletes the prior minor branch (e.g. 4.1). That could be done in cow.

@sminnee
Copy link
Member Author

sminnee commented Jun 18, 2018

That means we have a maximum of 1 minor release for major release running. I think right now we assume that we have 2.

  • Agree a rule on support times - 1 or 2 minor releases active for security patches?
  • Agree the specific process for end-of-support for a minor release - produce a final stable release and delete the branch?
  • Automated that process in cow.

The first 2 are the most important to do first. The 3rd can be done if and when we have the time & inclination but shouldn't be a barrier to the first 2. In particular, I would probably wait until the process is clear before implementing any automation, as otherwise we risk codifying the wrong process.

@robbieaverill
Copy link
Contributor

How do you expect this approach to affect the supported modules? Should we be aiming to keep the process the same over them as well?

@sminnee
Copy link
Member Author

sminnee commented Jun 18, 2018

That makes sense for consistency. Also, if we assume that people benefitting from supported modules will be drawing from the same "recipe" of supported modules, then it seems fine to test a supported module only against the 1 or 2 stable release lines, rather than testing every combination.

@robbieaverill
Copy link
Contributor

Yeah. It might mean more admin for changing travis config across all modules more often but hopefully we can automate it at some point

@robbieaverill
Copy link
Contributor

One main benefit I see is we can set default branches to the major version branch and have all contributions (non breaking) into that. We currently ask people a LOT to switch to minor branches in their PRs, which often results in confusion and the PR closed with a new one opened

@sminnee
Copy link
Member Author

sminnee commented Jun 18, 2018

That's currently the case on framework.

Also: maintainers can change the destination branch of a PR without needing the requester to do that.

@kinglozzer
Copy link
Member

My main concern with this approach is that we’ll increase the delay for bugfixes to be released.

Release Time since previous minor release
4.1.0 4 months (since 4.0.0)
4.0.0 N/A (major)
3.7.0 10 months (since 3.6.0) *
3.6.0 6 months (since 3.5.0)
3.5.0 4 months (since 3.4.0)
3.4.0 4 months (since 3.3.0)
3.3.0 4 months (since 3.2.0)

*arguably an edge case because I don’t think we originally intended to release a 3.7.0 at all.

If we assume 4 months is the typical timeframe between minor releases, are we suggesting that we only release bugfixes every 4 months? Or would we aim to increase the frequency of minor releases?

Assuming the latter, can we feasibly increase the frequency of minor releases while still (safely) accepting new API additions to minor releases? Or would they start being nudged towards major releases?

@dhensby
Copy link
Contributor

dhensby commented Jun 18, 2018

At the moment we have a lot of conversations about whether a change is semver compliant or not and which branch it should go in, I feel this has a risk of just changing the conversations to "is this critical or not"

I think @kinglozzer also touches on a good point. Does this just mean we accelerate our minor release frequency so that bugs that aren't critical but are mid - high impact are released?

This also would make it more critical for us to support a single minor release as a LTS so that people fixing to ~x.y.z actually were locked to something meaningful.

If we follow this through with a thought experiment, if we release 4.2.0 and then we find no critical issues and release 4.3.0 and then we find a critical issue in 4.3.0 and release 4.2.1 and 4.3.1, we can then release 4.4.0, delete 4.3 and find another critical issue, then anyone linked to ~4.3.0 now essentially has a meaningless constraint. (I hope that's easy enough to follow).

Something that may not be a concern of this ticket is how this is affected by support commitments/timelines to CWP recipes.

@dhensby
Copy link
Contributor

dhensby commented Jun 18, 2018

something I've seen with phpunit is that there are none of the branches that represent MAJOR.x-dev branches, there's only MAJOR.MINOR.x-dev branches. ie: no 3 or 4 branch, that could be an interesting alternative approach.

I'd also like to see a proposal that's more joined up with the support obligations of CWP. There's no point pretending that 3.4 is not supported if there's a commercial imperative that means it needs another 3 months support (as an imagined example). So I'd rather see a set out long term plan for releases that gives everyone clarity and certainty going forward.

@robbieaverill
Copy link
Contributor

Yep, true re: CWP. We're mostly in line with it now, but we have SilverStripe 3.5 via CWP 1.5 supported until 02/12/2018 and 3.6 via CWP 1.6/1.7/1.8 supported until 30/11/2019.

@sminnee
Copy link
Member Author

sminnee commented Jun 18, 2018

something I've seen with phpunit is that there are none of the branches that represent MAJOR.x-dev branches, there's only MAJOR.MINOR.x-dev branches. ie: no 3 or 4 branch, that could be an interesting alternative approach.

The implication of this is that master is 4.x-dev, and there's no place to put breaking APIs for the next wee while. I'd actually support this – at least for SSLtd-funded efforts – but it seems major enough to warrant a separate RFC.

@sminnee
Copy link
Member Author

sminnee commented Jun 18, 2018

The obligations of CWP are roughly that for any minor release we provide to CWP, we provide 18 months of security fixes. This isn't necessary every minor release; if not then we'd have some minor releases that were "semi-LTS" or "medium-term support".

Under the model described in this RFC, we could potentially increase the frequency of minor releases to 4-6 weekly, but only offer "medium term support" for some of them. "Medium term support" would be 18 months of security fixes. You could use a ~x.y.z with such releases and be comfortable that you're getting 18 months support.

For any other releases they would be supported only until the next minor release is produced, meaning that you should use a ^x.y.z requirement if you're running with that track.

It risks being a bit confusing, but in principle it would work. You could present it to users in terms of the composer expressions that they should use:

  • ^3: supported until December 2019
    • ~3.5.0: supported until December 2018
    • ~3.6.0: supported until November 2019
    • ~3.7.0: supported until December 2019
  • ^4: supported until November 2021
    • ~4.1.0 supported until November 2019

But, ~4.0.0 would be bad because that doesn't have medium-term support.

My goal with this RFC was to move direction of having more of a straight line of releases that we're working on, rather than a big tree. Any branches of the tree (i.e. security/critical fixes on supported release lines) would hopefully be exceptions that cover the minority of our dev work, rather than being a significant piece of effort.

So... would such a system achieve that?

@chillu
Copy link
Member

chillu commented Jul 9, 2018

Just for people which aren't so entrenched in composer symbology (like myself): ~1.2.3 is equivalent to >=1.2.3 <1.3.0, ^1.2.3 is equivalent to >=1.2.3 <2.0.0 ;)

Agree we need to simplify release line maintenance. 4-6 weeks for a minor release is suitable for bugfixes, but pretty short when you're thinking around introducing new APIs which might want to go through a beta phase before stabilising and becoming unchangeable under the shackles of semver ;) This could be covered by having some modules running a longer cadence (e.g. graphql). We've already decided that we're OK with version numbers going out of sync, e.g. silverstripe/installer@4.3.1 pulling silverstripe/graphql@3.2.3 and silverstripe/versioned-admin@1.1.6, right? In an ideal world, new APIs like this would be introduced in the next major release, but that's just not a feasible reality for us during the next two years.

It's a +1 from me if we're fine with the above: Modules with larger API additions will need to have (non-critical) bugfixes delayed in order to properly stabilize the minor release (rather than the patch release)

The implication of this is that master is 4.x-dev, and there's no place to put breaking APIs for the next wee while. I'd actually support this – at least for SSLtd-funded efforts – but it seems major enough to warrant a separate RFC.

I don't understand how that works - they alias to the latest minor release branch (currently 7.3.x-dev), and don't describe in their contributing guide how new major release features are supposed to be introduced. But yeah, separate RFC.

Under the model described in this RFC, we could potentially increase the frequency of minor releases to 4-6 weekly, but only offer "medium term support" for some of them.

We need to provide 18 months of security fixes for some release every quarter under CWP, so that's a fixed number (18 months / 3 month quarters = 6 maintained releases) regardless if that's an increment in the minor or patch release number.

@sminnee
Copy link
Member Author

sminnee commented Jul 9, 2018

I don't understand how that works - they alias to the latest minor release branch (currently 7.3.x-dev), and don't describe in their contributing guide how new major release features are supposed to be introduced. But yeah, separate RFC.

I interpret this as "you don't get to break public APIs until the core team decide to start aliasing to 8.x-dev".

@sminnee
Copy link
Member Author

sminnee commented Jul 9, 2018

4-6 weeks for a minor release is suitable for bugfixes, but pretty short when you're thinking around introducing new APIs which might want to go through a beta phase before stabilising and becoming unchangeable under the shackles of semver

I think that we should probably find a better way of addressing this than simply having a longer beta cycle

For example, perhaps we should get in the habit of marking any but the most trivial new APIs as “internal” in their initial release.

This has the benefit of providing everything else in the release to new users, without shackling ourselves to the api prematurely.

To put it another way, it’s kind of like a feature-flag for api stability ;)

I think this would be worth considering even if we didn’t do 4-6 week releases

@sminnee sminnee closed this as completed Jul 9, 2018
@sminnee sminnee reopened this Jul 9, 2018
@chillu
Copy link
Member

chillu commented Aug 27, 2018

I think we have two mutually exclusive goals:

  • Existing commitment: Maintain quarterly releases on CWP for 18 months. Most of these will be on a new minor release line, rather than patch releases on an existing minor release line.
  • Sam's proposal: Only maintain the latest minor release, to decrease maintenance complexity

As far as I understand, changing the former isn't up for discussion, although we could separate those existing contractual commitments from our open source maintenance comms - at the risk of inconsistent release management. My interpretation is that this support isn't satisfied through new minor releases (e.g. applying a security fix by upgrading your 4.2.1 release to 4.3.1 instead of 4.2.2).

Sam said: The obligations of CWP are roughly that for any minor release we provide to CWP, we provide 18 months of security fixes. This isn't necessary every minor release

In effect, this applies to every minor release at the moment (happening less than once per quarter). If we switch to a more frequent cycle of 4-6 weeks for a minor release, we'd have to distinguish which minor release falls under this support model. But the total number of supported minor releases is the same (18 months / 3 months = up to 6 minor releases). So I don't see how that's saving anyone any time in release management.

I'm concerned with going from ~4-6 months cycles in minor releases to 4-6 weeks. As Loz mentions, this short cycle is necessary under the proposed model to get bugfixes out in a reasonable timeframe (in absence of patch releases on older minor release branches).

For example, perhaps we should get in the habit of marking any but the most trivial new APIs as “internal” in their initial release.

This is discussed in more depth at RFC: Better handling of new APIs in minor releases. I think the proposed approach has merit for introducing APIs without years of delay until actual production usage, but it's getting a bit too messy when used to support 4-6 week minor release cycles - as a means to avoiding a beta phase during which the API can somewhat stabilise. "API not in a stable release yet" is a much stronger indicator for developers than "API in stable release, but marked as internal".

@chillu
Copy link
Member

chillu commented Aug 27, 2018

OK, here's a draft for our supported versions release docs. It mostly documents the status quo (not changing to 4-6 weekly minors), with a relatively weak statement on Sam's internal API proposal.

It introduces an inconsistency between public release comms ("Minor releases are supported for six months after their stable release"), and CWP comms ("most minor releases are supported for 18 months after their stable release"). This is a reflection of the fact that the CWP contract is not going to be around forever, and we're trying to set (more reasonable) expectations for a much wider community here.

Note that no matter what we do, it's going to be very hard to apply the same rules to all of our supported modules. Are we prepared to create dozens of new minor releases every three months? Maybe we should limit this to the "core recipe" as the main semver commitment, as opposed to a more ambiguous set of "core modules"?

Draft:

  • The status of major releases is determined by the roadmap
  • Minor releases of major releases in "active development" or "full support" are released roughly every three months, and are supported for six months
  • Minor releases of major releases in "active development" or "full support" are released roughly every three months, and their End-of-Life (EOL) is announced at least six months in advance
  • The latest minor release is supported as long as the underlying major release
  • API changes and major new features are applied to the master branch, to be included in the next major release
  • New APIs can be applied to the current minor release of major releases in "active development", but should usually be marked as "internal" APIs until they're considered stable
  • Enhancements are applied to the latest minor release of major releases in "active development"
  • Non-critical bugfixes are applied to all supported minor releases of major releases in "active development" or "full support"
  • Critical bugfixes and security fixes are applied to the all minor releases of major releases in "active development", "full support" or "limited support"
  • Any patches applied to older minor releases are merged up regularly to newer minor releases (in the same major release)
  • Any patches applied to older major releases are merged up regularly to newer major releases

@chillu
Copy link
Member

chillu commented Aug 30, 2018

@silverstripe/core-team What do you think about the draft above? Specifically, committing to maintaining minor releases in supported major releases for six months, and performing minor releases at least every three months. This is currently not specified by our process. In effect, it would mean that most of the time, we're maintaining at least four minor releases, but this could go up to six depending on major release line overlaps

Probably a bit easier to communicate in table form. In the short term, it'll be a bit messy:

Tag Date Proposed Support Until
3.5.x Nov 2016 (already unsupported)
3.6.x May 2017 Sept 2018 Mar 2019 (since we can't retroactively announce end of support)
3.7.x June 2018 Dec 2020 (end of 3.x LTS), unless we release a 3.8.x, in which case support ends in Dec 2018
4.0.x Nov 2017 Sept 2018 Mar 2019 (see above)
4.1.x May 2018 Nov 2018
4.2.x Jul 2018 Jan 2019 (assuming we're doing a 4.3.x)

But if you fast forward six months (March 2019), it'll look like this (leaving out unsupported releases):

Tag Date Proposed Support Until
3.7.x June 2018 Dec 2020 (end of 3.x LTS)
4.3.x Sept 2018 Mar 2019 (just ending support)
4.4.x Dec 2018 Jun 2019
4.5.x Mar 2018 Sept 2019

And fast forwarding a year (Sept 2019, making some assumptions about 5.x release timing, separate discussion):

Tag Date Proposed Support Until
3.7.x June 2018 Dec 2020 (end of 3.x LTS)
4.5.x Mar 2019 Sept 2019
4.6.x Jun 2019 Dec 2019
4.7.x Sept 2019 Mar 2019 (last release before going into limited support?)
5.0.x Sept 2019 Mar 2019 (assuming a 5.1.x follows)

Note that there'll be longer maintenance commitments under the current CWP govt contract (18 months for versions contained in recipe releases), but I'd keep that separate from our wider open source commitments. Think of it as a temporary bonus which we take care of internally by our release managers keeping an eye on which critical bugfixes and security fixes need to be backported further.

Update: Amended to Sam's suggestion around "announce EOL at least 6 months in advance"

@sminnee
Copy link
Member Author

sminnee commented Aug 30, 2018

I think that rather than saying "6 month support" we probably want to say "announce EOL at least 6 months in advance". Specifically this means 2 things:

  • We should be tight about announcing the EOL date as part of the release
  • For the releases that are still live at the moment, lets announce their EOL as end of Feb.

This would affect 4.0, 4.1, and 4.2 a little (add 1 month)

I also think that we should state our support timelines for 3.x and 4.x. The 4.x support timeline may be greater than the currently stated support timeline of all 4.x releases, unless we basically map forward our expectation of all the future minor releases, which might be too much pre-planning.

@chillu
Copy link
Member

chillu commented Aug 30, 2018

announce EOL at least 6 months in advance

Yeah, that's an elegant solution. These bullet points are our baseline agreement with the community, which we can choose to exceed. It's about expectation management. But I think this also means we need to add minor releases to our roadmap visualisation, since we shouldn't rely on people reading release announcements which also say "by the way, <minor-release minus 0.2> is EOL in six months".

@sminnee
Copy link
Member Author

sminnee commented Aug 30, 2018

I think that a "release monitor" can/should be part of a module (the sitesummarizer one, maybe?)

@chillu
Copy link
Member

chillu commented Sep 5, 2018

It sounds like there's no major concerns with this. In practice, I think we need to tighten up our triage, review and release process a bit to make this stick.

  • We need a stronger definition of a "critical issue" apart from security issues, because those should be targeted at the oldest supported minor branch on the supported major release branches (currently, under the proposed model, that'd be 3.6 and 4.0). Our contributing code guide defines a "impact/critical" issue as "Broken functionality for which no work around can be produced". Which feels a bit weak.
  • I'd say that any critical bug should be merged up to newer minor release branches right away (it's critical after all), which we'd need to document in our core committers guide. We'd also need clear guides to check if a PR is against the right branch.
  • We need better processes around merging up those branches, at the latest this should happen on release (see release process). This might be cow checking all supported branches
  • We need to clarify if this applies to supported modules or just the core recipe, and clearly define that in our supported module docs. My current feeling is that it's not feasible, and we should clarify that only the latest minor release branch is supported (status quo). There's a different discussion for how long we support major release lines of modules (probably as long as the core release lines they're compatible with). But keen to keep that separate for now.

Any concerns on the above?

@ScopeyNZ
Copy link
Contributor

ScopeyNZ commented Sep 5, 2018

My only recommendation would be merging up often. I'm not a core committer but on our "satellite" module repos if I merge a bugfix I try to merge it up immediately. This is helpful if you are familiar with the code after a review - you can deal with any conflicts that may occur with a little more knowledge.

Merging up when theres 3 or 4 bugfixes can be pretty frustrating if something random is conflicting.

To address the last point - I don't think we should commit to this for all the supported modules, but we should do it as a "best practice".

chillu added a commit to open-sausages/silverstripe-framework that referenced this issue Sep 5, 2018
@chillu
Copy link
Member

chillu commented Sep 5, 2018

Docs update at #8361 (which formalises the agreement reached here)

@chillu
Copy link
Member

chillu commented Sep 5, 2018

Minor releases of major releases in "active development" or "full support" are released roughly every three months, and their End-of-Life (EOL) is announced at least six months in advance

This implies that there's a standard way to announce this. I think we should create an announcement template for a forum post. And then actually maintain a table like I've outlined above on silverstripe.org/roadmap. Everyone OK with that?

@chillu
Copy link
Member

chillu commented Sep 6, 2018

Note to self: We also need to update silverstripe.org/roadmap to reflect these changes, and ideally come up with a way to update them once every three months without involving a designer

@robbieaverill
Copy link
Contributor

Is this ready to be rfc/accepted now?

@chillu
Copy link
Member

chillu commented Sep 11, 2018

Hard to track consensus here, but there's six 👍 on the original description - which means rfc/accepted to me

@chillu
Copy link
Member

chillu commented Aug 16, 2019

I don't believe there's anything left to do here, we've got new release policies as per above, and they're actively followed already (e.g. we're no longer committing to 4.2). Note that there's been some consensus with product devs here not to be too agressive when removing branches from the repo at the same time as EOLs kick in, because it's hard to automate that, and it breaks existing PRs by default in a way that's not easily recoverable for contributors (don't know the details, Robbie raised that)

@chillu chillu closed this as completed Aug 16, 2019
@robbieaverill
Copy link
Contributor

Deleting branches breaks two things:

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

No branches or pull requests

7 participants