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

SEP 14 - Changes to branching and release strategy #20

Merged
merged 3 commits into from Nov 13, 2019

Conversation

@thatch45
Copy link
Member

thatch45 commented Oct 1, 2019

This proposal is to modernize and streamline our branching and release strategy

@thatch45 thatch45 requested a review from saltstack/team-core as a code owner Oct 1, 2019
@pull-assigner pull-assigner bot requested review from Ch3LL and removed request for saltstack/team-core Oct 1, 2019
@waynew waynew changed the title Changes to branching and release strategy SEP 14 - Changes to branching and release strategy Oct 1, 2019
@waynew

This comment has been minimized.

Copy link
Contributor

waynew commented Oct 1, 2019

This will be SEP 14.

@dwoz
dwoz approved these changes Oct 1, 2019

## Versioning (Naming)

With the `neon` release, to indicate the new change in release process, Salt will change to a new, non-date based version schema beginning at 3000. The version will be MAJOR.PATCH. For a planned release containing features and/or bug fixes the MAJOR version will be incremented.

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Oct 1, 2019

why 3000, and not just use an epoch to indicate that you are going back to semver?

https://www.python.org/dev/peps/pep-0440/#version-epochs

This comment has been minimized.

Copy link
@dwoz

dwoz Oct 1, 2019

We discussed using epoch. However we're not going 100% semantic versioning. Instead of the major number indicating breaking changes, it's purely indicating features. Breaking changes will be handled by giving a 3 release (1 year) notice before they happen. Using the 3000, 3001, ect version scheme allows us to have a predictable release number for breaking changes.

I.E. We note in the change log of 3001 there will be a breaking change in 3004.

This comment has been minimized.

Copy link
@dwoz

dwoz Oct 1, 2019

@gtmanfred I realized my last comment did not fully answer your question. I thought you were talking about using and epoch date for the version. Not using a version epoch pointed out in the document you linked to. The primary reason for avoiding version epoch and using 3000 instead is to avoid any complications when packaging across the many distros and OSes. While we could likely find ways to make sure versions with epochs work with OSes like Windows. Avoiding that all together and going with 3000 is just and easier and more direct path forward.

This comment has been minimized.

Copy link
@OrangeDog

OrangeDog Oct 9, 2019

Why aren't you going for actual semantic versioning?

## Unresolved questions
[unresolved]: #unresolved-questions

Hopefully all the questions have been answered in this SEP. Upgrading Salt should continue to work like it always has - our changes are focused on the external development process of Salt. If you feel like you have unanswered questions, please come ask them at the Salt Office Hours on October 1st, 2019, or find us in [#salt on IRC on Freenode](http://webchat.freenode.net/?channels=salt&uio=Mj10cnVlJjk9dHJ1ZSYxMD10cnVl83), the [SaltStack Community Slack](https://saltstackcommunity.herokuapp.com/), or the [Salt Users mailing list](https://groups.google.com/forum/#!forum/salt-users).

This comment has been minimized.

Copy link
@cachedout

cachedout Oct 2, 2019

Contributor

I think the biggest unanswered question I have is around the topic of deprecations.

As I understand this proposal, majors are released on a 90 day schedule. If a need arises for a minor, say because of a critical bug fix, then one will be issued at some point during the 90 day cycle, though this isn't always the case, and ideally (?) is never the case.

So, assuming an ideal cycle where there are four releases a year, it seems like deprecations could happen at any of those major releases. (Please correct me if this is wrong.)

So, I have to wonder if this is setting ourselves up for a situation wherein most, or even all, upgrades are between majors, and so, even to to take in minor bug fixes, users are exposed to deprecations and a resulting change in behavior. I think we all agree that avoiding such a bind is desirable.

Could there be room in a single-branch strategy for a major-minor model to still exist? For example, what if we took the four releases a year and structured them this way?

Quarter Type Policy
Q1 Minor (breaking changes NOT allowed)
Q2 Minor (breaking changes NOT allowed)
Q3 Minor (breaking changes NOT allowed)
Q4 Major (breaking changes ARE allowed)

IMHO, this keeps most of the benefits you've described in this proposal but it also allows users more predictability when it comes to upgrades. WDYT?

This comment has been minimized.

Copy link
@waynew

waynew Oct 2, 2019

Contributor

With the increased release cadence in order to allow proper planning for feature deprecation, Salt will introduce a minimum 1-year, expected 3 major release cycle. If neon ships in January 2020, any new deprecations will not be removed until January 2021 at the earliest.

I mean, I guess the answer is - yes there could be breaking changes in any release (which is not different from the current major release process). But they won't be "surprise here is something that you've never heard of!" kinds of changes, just ones that have seen a minimum of 1-year of warnings calling out the deprecation.

I expect we would require longer lead times for more invasive breaking changes.

Thinking about the alternative that you propose where we have a once-per-year breaking change release, and feels icky to me.

Initially I thought that could make sense, but the problem with that is that if we have one breaking change release per year we literally have to break everything that we're going to break in that release.

Instead of making minor changes over time - many of which will likely affect a subset of our user base, we would be making different changes at one time that would likely affect our entire user base.

The other side effect there is that there would be some kind of campaigning closer to the deprecation release to do things that would break for people with less warning. If we simply state that every deprecation needs at least a year of warning, that puts everyone on a level ground, and there's no temptation to "just make an exception this one time".

That may be a small side effect, but it is still there.


Ultimately, I prefer the potential for breaking changes with each release - this spreads out the potential issues: if there's something that wasn't caught in testing or RC with a deprecation, the hope is that it will be a minor issue, and we'll be able to get it out in the patch release. Worst case it will be super gnarly to fix and we'll just have to roll back that change in the patch release. But either case should cause much less impact overall.

Compare that to the case where we may have several deprecations go wrong in a single release. All of the sudden the world is on fire for everyone, and the core team is running around trying to fix all of the things. Ick.

I prefer small changes over time, to sweeping changes all at once (but, maybe I'm in the minority 🤷‍♂).

@mchugh19

This comment has been minimized.

Copy link

mchugh19 commented Oct 2, 2019

If the current process is to deprecate warn the release before breaking (giving a year or so warning), perhaps in the new more releases a year model, that could be updated to deprecate warn for 3 releases (retaining the year or so warning) before breaking?

@waynew

This comment has been minimized.

Copy link
Contributor

waynew commented Oct 4, 2019

@mchugh19 we're removing the release # requirements - it may be 3 releases, it may be 12 (though that's unlikely 🙃 ) and strictly moving to a time frame.

That means regardless of the number of releases we make per year, you can always rely on getting at minimum a year of warning before your feature is removed. This will also be called out in our changelog (see SEP 1), both when the deprecation warning is added, as well as when the actual feature is removed.

So if we cut 3 releases per year, yes, we will warn for 3 releases. If we cut 10 then we will warn for 10 releases. Seem reasonable?


## Versioning (Naming)

With the `neon` release, to indicate the new change in release process, Salt will change to a new, non-date based version schema beginning at 3000. The version will be MAJOR.PATCH. For a planned release containing features and/or bug fixes the MAJOR version will be incremented.

This comment has been minimized.

Copy link
@plinkable

plinkable Oct 8, 2019

I'm wondering why the version number should change in this manner rather than simply sticking with date-based versions..

Right now the code names are meaningless to me and I have to trundle off to the website to look them up. Making the version number meaningless doesn't seem to add anything, just make it more complicated to understand which version is which. Perhaps for the first few releases we can keep track in our heads, but after 10 or 20 it's going to be a pain to work out just how old a particular release is.

I realise most packages don't use dates, and it's not a problem for them, but salt does, and it's been helpful to know - without looking anything up - just how old someone's version is when trying to help them with a problem. I guess this is mostly a community support request rather than anything, but I wanted to make sure it was at least noted and hopefully even explained before being set into stone.

@max-arnold

This comment has been minimized.

Copy link

max-arnold commented Oct 8, 2019

The SEP doesn't mention a feature freeze period; however, during the last office hours, it was said to be ~1.5 months (1 before and 0.5 after a release).

Maybe I misunderstood the exact branching strategy, but it looks like the master branch would be feature frozen. This feels a bit long compared to 4 month release cadence.

A backlog of unmerged features means that they aren't integrated together, and although individually the PRs could be green, lots of conflicts and test failures are expected during the merge stage.

You might want to consider making a temporary RC branch to prevent the backlog and avoid blocking the master branch from merges.

@OrangeDog

This comment has been minimized.

Copy link

OrangeDog commented Oct 9, 2019

As a community contributor I can usually find and fix simple issues or provide my custom workarounds (because that's my job), but I don't have the resources to dive into the test suites.
Will the core team be adding tests and other admin work for PRs like this?

@waynew

This comment has been minimized.

Copy link
Contributor

waynew commented Oct 9, 2019

@max-arnold

A backlog of unmerged features means that they aren't integrated together, and although individually the PRs could be green, lots of conflicts and test failures are expected during the merge stage.

I think the biggest problem would just be a lengthy cycle (e.g. a month-ish) to merge the changes.

There is definitely some (understandable) caution around making the release branch, but one approach that we could take is to create the release branch at the moment of RC (one month before release).

Then we would only merge bugfixes (preferably issues for which there are no workaround) into RC, while we could still merge new things into master.

What concerns me the most about this approach is the practicality around the bug fixes. Will we:

  • make PRs against the release branch, and wait to merge into master?
  • make PRs against both branches?
  • make PRs against one branch and cherry pick into the other?

It sounds like there is going to have to be a tradeoff here - which one are we most comfortable with? Delaying feature PRs periodically? Some extra overhead in merging things?

Perhaps there's other, more simple solutions that I haven't encountered. If not, what trade off are we most comfortable with?

@waynew

This comment has been minimized.

Copy link
Contributor

waynew commented Oct 9, 2019

As a community contributor I can usually find and fix simple issues or provide my custom workarounds (because that's my job), but I don't have the resources to dive into the test suites.
Will the core team be adding tests and other admin work for PRs like this?

I think that if we don't have time (or inclination) to write regression tests for issues that we're fixing, it's probably a good idea to call that out. I've found that writing tests for Salt is a great way to learn more about what it's doing in any particular place. We could definitely add the Help Wanted label.

For bugs that are more integral to Salt, I'm sure that the core team could take that on. Though it's definitely better if the contributor who is most recently familiar with the issue would write the tests.

For bugs in some less popular areas I'm sure it makes sense to expect some push back 🙃

@cachedout

This comment has been minimized.

Copy link
Contributor

cachedout commented Oct 12, 2019

make PRs against both branches?
make PRs against one branch and cherry pick into the other?

It should be a PR created on the RC branch which is made up from cherry-picked commits against the master branch. IMHO, a system of labels should also be created to help manage this process so we know what needs to be backported and what remains.

Here is a tool that will do all of this for you: https://github.com/sqren/backport

@waynew

This comment has been minimized.

Copy link
Contributor

waynew commented Oct 14, 2019

tl;dr - I was concerned, but now I'm a huge fan of the feature freeze, because back-of-the envelope calculations.


It should be a PR created on the RC branch which is made up from cherry-picked commits against the master branch. IMHO, a system of labels should also be created to help manage this process so we know what needs to be backported and what remains.

That sounds like what we used to have... and what we've seen is that it becomes impossible to track down what problem was introduced when, where, and why. It also becomes easy to lose track of what needs to be where.

If we simply say, "Code isn't shipped unless it's in master," then we eliminate all of those problems - for people trying to push code, for people trying to maintain the project, and people trying to use the code.


I think that this feature freeze is actually a good thing.

If what we're saying is that bug fixes can be merged (with tests) at any point then Salt should always be getting more stable. Bug fixes will continually get merged in during the the entire process.

If the feature PR continually breaks with every subsequent bugfix PR, that's an indication that we need to pay special attention to it because it's causing massive changes across the codebase. We probably need the month and a half to give it a proper review, build some manual test cases, etc.


To imagine a worst case scenario, let's say that 50 features are going to get merged into Salt for the Aluminum release. Let's say we're at a 3-month cycle by that point. If we remove the 2 weeks post-release and the 4 weeks pre-release that gives us 8 weeks, or about 7 features per week. Since each PR has to be built on the most recent master with passing tests, that means that each time we merge, we're going to have to wait for a rebuild before we can merge the next one. Realistically that means we might be able to merge one in the morning, one in the evening. So, we probably could merge 7 features per week - in theory assuming no merge conflicts and all passing tests we could even do up to 14, giving us a(n extremely theoretical) capacity of 100 features shipped per release.

I was actually kind of concerned about the 1.5month wait, but now that I've done the math I'm not really concerned anymore. In the past it's not like we were able to deliver a shorter release cycle (in fact, we've been pretty good about missing releases and throwing features in point releases because it's not that big of a feature). We're not taking away anything that we were able to deliver on before, in fact we're probably going to be able to deliver more features faster just by sticking the feature freeze into effect. The other advantage is that by giving ourselves a month to fix bugs, if a feature is accepted a little prematurely we stand a much better chance of catching it and fixing the problems before it gets into an official release.

@cachedout

This comment has been minimized.

Copy link
Contributor

cachedout commented Oct 14, 2019

That sounds like what we used to have... and what we've seen is that it becomes impossible to track down what problem was introduced when, where, and why. It also becomes easy to lose track of what needs to be where.

Could you please help me understand where the trouble would be? If all original contributions are to master and all cherry-picks are from master, couldn't you always bisect master to find any regression?

@waynew

This comment has been minimized.

Copy link
Contributor

waynew commented Oct 14, 2019

That's a fair point.

But it's also just adding an extra step that we could simply not do. What are we gaining by removing the feature freeze period?


One major challenge for the community is understanding changes coming in the next release. Focusing efforts on a single branch will help with this, but additionally Salt will create a changelog file to be updated by contributors, that will be used to document changes in a human-readable way, as specified in [SEP 1](https://github.com/saltstack/salt-enhancement-proposals/blob/master/accepted/0001-changelog-format.md). Updating this documentation will be required as part of the merge process - either from the contributor or by a Salt core team member.

Salt [documentation](https://docs.saltstack.com) will be updated to point to the most current release (2019.2.1 at the time of this writing). For users who have not yet upgraded, or those who are testing the unreleased version of Salt, Salt take an approach similar to the official Python documentation, and have a drop-down or some other way to easily select documentation for other Salt versions.

This comment has been minimized.

Copy link
@twangboy

twangboy Oct 16, 2019

Salt will take an approach

## Support matrix

To be able to focus on the stability and innovation in the Salt platform, we will be adopting the industry-standard approach of no longer supporting older releases. Salt will provide select support for serious bugs and CVEs for the most recent release. Minor bug fixes will be targeted in the next scheduled Salt release.
Comment on lines +132 to +134

This comment has been minimized.

Copy link
@dafyddj

dafyddj Nov 11, 2019

I think some clarification is needed here. On the Nov Office Hours I asked a question re. this and the answer was that the product support lifecycle wasn't changing.
So, will older releases be supported (however limited) or not?

@waynew
waynew approved these changes Nov 12, 2019
@waynew

This comment has been minimized.

Copy link
Contributor

waynew commented Nov 13, 2019

Looks like we've got our 5 approvals 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.