Consider no longer using the PR consistency checking script for astropy core (or rethink)

[astrofrog]

While working on doing a v5.0.5 release I have noticed that the PR consistency scripts now return a lot of issues that are not issues:

• When maintaining 5.0.x and 5.1.x only the changelog rendering from 5.1.x gets forward-ported to main, so e.g. a PR merged labelled as 5.0.4 will actually not appear in the 5.0.4 section of the changelog if it was also backported to 5.1.x, but instead in the next 5.1.x release (we were ok with this decision back when we set up towncrier, I just mean that the scripts here don't recognize that).
• The changelog rendering happens during the release process and so the scripts can't easily check right now the rendered changelog until the last minute - currently this causes a lot of warnings about missing changelog entries.

In addition I spent a while just adding manual merges to the script, but haven't actually encountered any real issues.

Now that we have a pretty robust system for backporting and generating the changelog, and that we check for changelog entries being present in PRs at the time when the PR is open, and furthermore that we can just check for PRs labelled 'Still Needs Manual Backport' I don't think the scripts here are really needed anymore (for the core package).

Would we be happy to switch for astropy core to just saying that one should check for PRs still requiring manual merging? This would drastically reduce the burden of doing bugfix releases.

If we want to continue using the scripts, we are going to have to rethink how the checks are done re: LTS releases and so on.

[pllim]

Let's break this down and see...

Current situation:

• 2 backport branches.
• Automated backport by bot or manual backport by human.
  • Bot backport follows a certain pattern and easy to script for.
  • Human backport might or might not follow the instructions prescribed by bot.

Possible scenarios (might not be exhaustive):

• PR milestoned to LTS
  • Auto backports to both branches exist but (the both copies milestone blindly, so even the backport to 5.1.x will have 5.0.x milestone the way we milestone the original PR)
    • unmerged
    • only one of the branches merged
    • both merged
  • Auto backport exists only for one branch (the other one missing due to conflict)
    • Manual backport PR exists and merged
    • Manual backport PR exists but unmerged
    • Manual backport PR does not exist (no human bothered) -- At this point, do we just say we won't backport or find someone to do it? If we decide not to backport, the a human needs to go back to original PR to re-milestone it.
  • No auto backport PRs exist (both branches have conflict)
    • (This would invoke a combo of "manual" possibilities from above for one or both branches)
• PR milestoned to only last release branch (not LTS)
  • Auto backport exists
    • unmerged
    • merged
  • No auto backport PR exists
    • Manual backport PR exists and merged
    • Manual backport PR exists but unmerged
    • Manual backport PR does not exist (no human bothered) -- At this point, do we just say we won't backport or find someone to do it? If we decide not to backport, the a human needs to go back to original PR to re-milestone it.

[pllim]

Also... sometimes people backport by pushing it directly to the backport branch, right? Instead of opening PRs.

[saimn]

I used the PR consistency scripts for the 5.1 release, both to see what it was doing and to cross check that the changelog was ok. It was useful to see that everything was consistent, but I could have done without it.
The issue is that it is not so easy to check that all PRs have been backported, because of the issues mentioned by @pllim above. But I'm not sure if there is an easy way to improve that, and we could also choose to do releases with the PRs that have been backported and that's it.

[bsipocz]

we could also choose to do releases with the PRs that have been backported and that's it.

That's a valid idea, but in that case, their milestones must be updated to reflect that the PRs were, in fact, not backported.

[astrofrog]

I guess at the end of the day:

• If a PR can be automatically backported then the only issue is that the PRs might not be merged yet so need to check for that
• If a PR cannot be automatically backported to at least one branch then it will have the 'Still Needs Manual Backport' label until someone actually does the backport(s) and removes the label

So at release time it should be sufficient to check for:

• Unmerged auto backport PRs
• PRs with the 'Still Needs Manual Backport' label

and that should be sufficient? Then the only real failure mode I can think of is if someone removes the 'Still Needs Manual Backport' label when in fact the backport was not done correctly or only one of two was done. So the question is whether we want to find a way to guard against that - but the issue is that as they are at the moment, the scripts don't handle manual merges well unless we explicitly list them which is time consuming.

In any case, checking the changelog (the two main issues I mentioned at the start of this issue) is something that doesn't work well anymore because of how we handle LTS but the flip side is that because of the towncrier workflow it's very unlikely that there would be an inconsistency between what is backported and what ends up in the changelog so that consistency doesn't need to be checked.

[saimn]

Seems a good summary, I agree that with a query on the labels and milestone we should be able to spot issues easily. The only caveat is that when backporting to both 5.0.x and 5.1.x, the PR backport for 5.1.x will have 5.0.x as milestone (because it inherits the milestone from the parent PR). It's easy to fix manually when checking the list of PRs in the milestone page.

[pllim]

As @bsipocz pointed out above, we still need a script to check if the original PR milestoned to be backported need the milestone changed if the backport somehow didn't make it to the actual bugfix release?

the PR backport for 5.1.x will have 5.0.x as milestone

Yeah, at first I tried to manually correct that but then I gave up because it was too much work to keep track especially when other maintainers also merging them without changing inherited milestone. 😅

[pllim]

At 2023-02-28 infrastructure tag-up, those present expressed a need to brainstorm more on the alternatives first.