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

DOC: Formalize LTS backport policy #11429

Merged
merged 4 commits into from Oct 19, 2021
Merged

DOC: Formalize LTS backport policy #11429

merged 4 commits into from Oct 19, 2021

Conversation

pllim
Copy link
Member

@pllim pllim commented Mar 25, 2021

Description

This pull request is to formalize LTS backport policy.

We have to decide where to best place such a policy and hammer out the wordings.

Fixes #11405

@github-actions
Copy link

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

@pllim pllim added the Release label Mar 25, 2021
Copy link
Member

@Cadair Cadair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve this message.

*******************

Starting from astropy 5.0, backports to Long-Term Stable (LTS) releases
will only cover:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After the standard support life cycle.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cadair , can you please elaborate on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well why the LTS version is both "the LTS" and the latest release the backport policy is the same as normal?

docs/lts_policy.rst Outdated Show resolved Hide resolved
@pllim pllim modified the milestones: v4.3, v5.0 Apr 19, 2021
@pllim pllim marked this pull request as ready for review April 19, 2021 17:05
@embray
Copy link
Member

embray commented May 17, 2021

I think this is well stated, thank you.

@pllim pllim requested a review from saimn May 17, 2021 16:57
@saimn
Copy link
Contributor

saimn commented May 18, 2021

After some thought I tend to disagree, or at least that not what LTS means for me. As a comparison, Python releases have bugfix releases during 2 years, and then security/critical fix releases when needed during 3 more years. Changing LTS to security/critical/on-demand fixes only is a regression imo, and not what LTS was intended for, and also not what we have been doing since LTS started.
I understand the maintenance cost but I think that we could reduce this cost with a better backport/release procedure. Also backporting bugfixes is often not that difficult in terms of conflict since bugfixes usually impact limited sections of the code. At least with my io.fits hat, I think that bugfix backport are usually not complicated, and are of great value for LTS users. If we want to stop doing that, it would mean for me the end of LTS. And about backporting security/critical fixes, there is no need for a LTS branch for that. I guess if there was some security/critical fix (which is not so frequent with our codebase, not web oriented etc.) we would try to backport it to any recent enough version we can anyway.

@embray
Copy link
Member

embray commented May 21, 2021

@saimn I agree with your points overall, and it's how I feel as well. But I didn't really want to push back on this because I know LTS maintenance has been a big burden for all involved, so I'm sympathetic to the desire to change the policy. I know I just wrote earlier that I think this policy is well-said, you have me wanting to backtrack a bit.

I think the backport procedure usually worked pretty well in the past but over time became more complicated for various reasons (mostly to do with CI I think). Now that we are experimenting with meeseeksbot and also have CI a little better straightened out for the 4.0.x branch (#11123) I think we should wait and see if it becomes any easier.

Maybe what we could do is soften this slightly with wording like:

Bug fixes that are trivial to backport, and critical bug fixes.

Where we can define "trivial to backport" as either "automated backports merge / pass CI without any additional intervention" or it would take me at most 5 minutes to resolve any minor merge conflicts. And by "me" I mean I'll volunteer.

@saimn
Copy link
Contributor

saimn commented May 25, 2021

Indeed, the backport workflow with the bot should help by backporting just after PRs are merged (so changes would still be fresh, and it would be up to the sub-package maintainer to see if it's worth fixing the conflicts) and by running CI on the backport branches more regularly.

@pllim
Copy link
Member Author

pllim commented May 26, 2021

So, uh, how do we proceed? Should I just close this without merge?

@pllim pllim requested a review from eteq June 14, 2021 17:21
Copy link
Member

@eteq eteq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some wording suggestions based on some out-of-band discussion.

My two cents is that with the backport bot things should be a bit easier, so "critical" is not mandatory. But it is good to have this here nonetheless to make sure everyone understands that the purpose of LTS and the scope of fixes.

docs/lts_policy.rst Outdated Show resolved Hide resolved
docs/lts_policy.rst Outdated Show resolved Hide resolved
LTS Backport Policy
*******************

Starting from astropy 5.0, backports to Long-Term Stable (LTS) releases
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Cadair , you mean like this?

Suggested change
Starting from astropy 5.0, backports to Long-Term Stable (LTS) releases
After the standard support life cycle, backports to Long-Term Stable (LTS) releases

@pllim
Copy link
Member Author

pllim commented Jun 15, 2021

Update:

  • I applied new wordings from Erik, so should be less controversial now?
  • Still not sure what Stuart wanted in his comment above.
  • Should I move this to the section where it would only be rendered in latest and not stable?

@embray
Copy link
Member

embray commented Jun 16, 2021

Should I move this to the section where it would only be rendered in latest and not stable?

I don't know--if that's the policy starting v5.0 I don't see why it shouldn't be in the stable docs. I think we want to defend this policy as "stable" and not be pressured into changing it. If it absolutely must be changed, I think any change to the policy should coincide with a new release. But as it is I'm pretty sure this covers all the bases, including the "pipelines that absolute need it can have backports but they must be willing to help".

* Critical security fixes.
* Bug fixes where backporting is straightforward (e.g., no conflicts).
* More complex fixes are permissible only as resources allow, ideally with
backports done by the original author or from institutions who are on LTS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about "users"? In most cases, those users are institutions, but I don't think that we should write a policy that explicitly excludes non-institutional users.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added "users". Look better now?

docs/lts_policy.rst Outdated Show resolved Hide resolved
@pllim
Copy link
Member Author

pllim commented Jul 6, 2021

Hmm looks like I need to rebase to get rid of failures.

pllim and others added 4 commits July 6, 2021 18:00
as Stuart Mumford suggested. [ci skip]
[ci skip]

Co-authored-by: Erik Tollerud <erik.tollerud@gmail.com>
Apply suggestion from hamogu

[ci skip]

Co-authored-by: Hans Moritz Günther <moritz.guenther@gmx.de>
@pllim pllim merged commit 086633e into astropy:main Oct 19, 2021
@pllim pllim deleted the doc-lts-policy branch October 19, 2021 13:03
@pllim
Copy link
Member Author

pllim commented Oct 19, 2021

We can further word-smith this in follow-up PR(s). Thanks for the reviews!

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

Successfully merging this pull request may close these issues.

DOC: Have a formal policy on what goes in LTS or not
6 participants