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

Remove limitation on the frequency of semver-minor LTS relesease #772

Open
targos opened this issue Aug 16, 2022 · 13 comments
Open

Remove limitation on the frequency of semver-minor LTS relesease #772

targos opened this issue Aug 16, 2022 · 13 comments

Comments

@targos
Copy link
Member

targos commented Aug 16, 2022

I'd like to propose we remove the unofficial rule we have limiting the frequency of semver-minor LTS releases to one per quarter (I think?). Having prepared some of those releases, I think the rule is counterproductive, error-prone, and puts a huge burden on the releasers. Here are a few reasons I would like to remove the rule and let the releaser decide each time whether they are going to prepare a minor or a patch release:

  • The main branch is not managed in a way that facilitates our work. Once a few semver-minor changes have landed, a lot of subsequent semver-patch changes will depend on them, making it difficult to find commits that can land cleanly on the LTS branch. The consequence of this is that we tend to have relatively small patch releases followed by a huge minor release like v16.17.0 today.
  • Because most of the semver-patch commits cannot land cleanly without their semver-minor dependencies, the release preparation takes a very long time as the releaser has to check for each conflicting commit why it doesn't apply. Then they either add the appropriate label (dont-land, backport-blocked, backport-requested) or fix the conflict manually. Each manual fix takes time and may introduce bugs even if it seems trivial.
  • Users have to wait many months for changes they care about (many of these changes being semver-patch bug fixes) to land on LTS.

@nodejs/lts

@MylesBorins
Copy link
Member

I think this is a reasonable change to make. The biggest reason for this "rule" was due to customer complaints about frequency of minors on LTS. In particular cloud vendors are much slower to update minors for LTS than patch releases. We have even had folks complain about us doing minors at all.

The only thing I would point out is that Semver-Minors do tend to need a bit more baking time than patches, so the "rule" did enforce waiting a bit more time. I'm not sure we want to land Semver-Minor changes on LTS 2 weeks after landing in a Current release... so the problem regarding LTS having a mismatch to current won't necessarily be solved

@BethGriggs
Copy link
Member

There's a historic proposal with some discussion/rationale in #449 that may still be relevant.

I think I'd be +0 towards more frequent minors (mostly to help reduce conflicts).

Ref the concern around losing the additional baking time: with Active LTS releases being scheduled monthly, maybe to mitigate that concern we could extend the general baking rule to:

All commits should have baked in the current release for 1 month before landing in an Active LTS release.
*Our typical exceptions for severe bug fixes or urgent security-related patches would still apply.

The motivation for that suggestion is that, to me, it feels like it would be an easier policy to articulate to the wider-collaborators and end users who are often asking when things on current can land or will be shipped in LTS.

@danielleadams
Copy link
Member

I am +1 to more frequent semver minor releases (without the patches in between) and +1 to allowing a 1 month/4 week bake LTS bake time in Current. I don't have a lot of historical context, but the semver patch releases of the Active LTS are by far one of the more painful tasks of releasing.

The biggest reason for this "rule" was due to customer complaints about frequency of minors on LTS. In particular cloud vendors are much slower to update minors for LTS than patch releases. We have even had folks complain about us doing minors at all.

For those that want less frequent or no semver minor releases of supported Node.js releases, is the LTS Maintenance mode version an option?

@mhdawson
Copy link
Member

+1 from me on the proposal along with the suggestion of 1 month/4 week bake time.

I personally think SemVer minors are often lower risk than SemVer patch changes and those two together would preserve/improve the benefits of baking in Current while simplifing the release process.

@ruyadorno
Copy link
Member

I am also a big +1 from the point of view of a releaser. LTS releases are super demanding to work on (as noted by @danielleadams) and as a matter of fact I can't (in the current state of things) volunteer to work on them since it would exceed by far the amount of hours I can put on contributions in the scope of my current role.

Given that the project has been expanding the number of collaborators (and the Release team has been growing as well) I would love to see us going in a direction where we cut LTS releases more often - thus making the task simpler each time and enabling other releasers (in a similar position as I) to contribute more often.

@BethGriggs
Copy link
Member

I would love to see us going in a direction where we cut LTS releases more often - thus making the task simpler each time and enabling other releasers (in a similar position as I) to contribute more often.

I think our baking time restrictions on commits would limit how frequent it would make it worthwhile to do releases.

Traditionally, we've been very conscious of our downstream consumers of LTS releases (also part of the 'no releases on Friday' motivation). From talking to various consumers - updating releases takes a lot of work and less frequent releases is one of reasons that they opt to use LTS rather than Current.

I am not convinced having more frequent LTS releases would make things that much easier - as there will still be a lag between Current and Active LTS due to the baking time, and we'll experience a similar level of divergence in history (because of missing semver-major or do-not-land-x labelled commits in Current). The benefit is smaller releases, but that's at a cost to our downstream consumers.

@ruyadorno
Copy link
Member

The benefit is smaller releases, but that's at a cost to our downstream consumers.

Don't get me wrong, I respect very much the downstream consumers and I'm sure we all have the best intentions when we put them first but personally I'm very much on the side of changing the frequency in order to benefit the release team at a cost to the downstream consumers.

IMO the current frequency is very overwhelming to releasers (given to many factors, one of them being the longer hiatus between releases) and I would be very happy to see some change in a direction that could at least try to alleviate a little. It doesn't need to be drastic, and I would defer to the more experienced folks such as @BethGriggs to chime in on what that cadence could be changed to.

My concern is that the bar is currently too high for releasers and as a result of that less people ends up being able to contribute, putting even more pressure on the current folks taking LTS releases. I'm really afraid that ends up burning out people and driving even less people to take on the LTS releases.

@BethGriggs
Copy link
Member

My concern is that the bar is currently too high for releasers and as a result of that less people ends up being able to contribute, putting even more pressure on the current folks taking LTS releases.

I think that @targos suggestion of not limiting semver minors will lower the bar and reduce some of the burden (hopefully minimising conflicts and reduce how much time we spend dealing with commits that end up being blocked on the queued up minors).

IMO the current frequency is very overwhelming to releasers (given to many factors, one of them being the longer hiatus between releases) and I would be very happy to see some change in a direction that could at least try to alleviate a little. It doesn't need to be drastic, and I would defer to the more experienced folks such as @BethGriggs to chime in on what that cadence could be changed to.

At present, and for the past few years, we have aimed for monthly Active LTS releases. I personally think that's a reasonable cadence to keep considering the minimum baking times of commits on Current, downstream consumers, predictability, and releaser availability. If we do decide to increase the minimum baking time of all commits to 1 month, I think that would be additional rationale to keep to monthly LTS releases.

The next suggestion of cadence would likely be every two weeks - the same as for Current. In the past, we have omitted or delayed some of the two-weekly Current releases due to lack of releaser availability (which is completely reasonable as many of us are volunteers who have other commitments). I'm not convinced we should be setting out to increase our workloads/commitments by increasing the number of volunteers we need to find each month for Active LTS releases when the small group of active releasers have been overburdened in the past.

Personally, my ability to pick up releases wasn't impacted by how long I expected them to take (which can vary a lot, even for Current), I would have still only picked up n releases per quarter (n=0 while on hiatus as I am now 😅).

@ruyadorno
Copy link
Member

We discussed this again today in the WG meeting and everyone seems to be onboard with the idea of removing the limitation for landing semver-minor changes in monthly LTS releases.

@BethGriggs summarized the tweaks we would be doing pretty well, I'll try to list them here but feel free to intervene if I'm wrong or forgot anything:

  • Keep currently tentative monthly cadence for LTS releases
  • Remove semver-minor limitation, we just prepare LTS releases in a similar fashion to Current
  • Add an extra rule that allows for a 4-weeks period of baking time for commits that landed on Current to land on LTS, effectively releasers will be running branch-diff against the second-to-last published version of Current (e.g: today the latest version of Current is v18.8.0, so if I'm to prepare a LTS release I would cherry-pick commits from the v18.7.0 tag instead of v18.x branch)

I'm a big +1 to these changes 👍

One important thing to keep in mind is to document these changes well in the Release guide, I'll be the first one to forget about the baking time (brach-diff tweaks) otherwise 😅

@targos
Copy link
Member Author

targos commented Aug 29, 2022

One important thing to keep in mind is to document these changes well in the Release guide, I'll be the first one to forget about the baking time (brach-diff tweaks) otherwise

+1, btw I have always used a Current tag as the branch-diff reference for LTS, never v18.x.

@ruyadorno
Copy link
Member

btw I have always used a Current tag as the branch-diff reference for LTS, never v18.x.

Oh that's interesting, I'd be curious to hear from @MylesBorins or @BethGriggs if historically that has been the case, I have always assumed we would only cherry-pick from Current in order to avoid landing things on LTS that hasn't landed on a current release line among other potential issues.

I know it's a bit off-topic to this issue but it could be a good thing to document how to use branch-diff for LTS in the Release guide.

@targos
Copy link
Member Author

targos commented Aug 29, 2022

I have always assumed we would only cherry-pick from Current in order to avoid landing things on LTS that hasn't landed on a current release line among other potential issues.

That's true, but given how our release process works, a tag like v18.2.0 is guaranteed to point to a commit that lives in the v18.x branch. I prefer using a tag as I can be certain that the release is at least 2 weeks old. v18.x could be a release from yesterday.

@ruyadorno
Copy link
Member

From the conversation on the last Release WG meeting, I believe the last step here is to double check if any documentation is pointing to that unofficial semver-minor-on-LTS quarterly rule and update it accordingly.

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

6 participants