My bad adding a milestone is not editing apparently... :(

My bad adding a milestone is not editing apparently... :(

The milestone asks for a synchronization then to be rechecked: I don't think this is an issue, pull requests are rarely merged as is.

Do we have a doc page or a wiki page where we should mention some specificity regarding affecting milestones and the usage that we will do from them (I am mainly thinking about the release for the moment)

Not yet but I was planning to add a milestone page here.

OK so nothing is requested in the PR since this is the wiki page.

Thanks @thomasjpfan ! Indeed too many copied-pasted lines on my side... sorry...

Indeed too many copied-pasted lines on my side... sorry...

It is okay!

The code looks good. I have a question about how you see us using milestones for changes in CI.

For example, this PR is milestoned for 0.24.2, but it is a CI change that would only need to be placed on main. This PR does not need to be part of the 0.24.2 release.

Maybe we should should only check for milestones for PRs that require a changelog? We can use the same check we did in check-changelog.yml:

For example, this PR is milestoned for 0.24.2, but it is a CI change that would only need to be placed on main. This PR does not need to be part of the 0.24.2 release.

You are right, this is only needed in main... this was me trying to hurry things ... changing right now

Maybe we should should only check for milestones for PRs that require a changelog? We can use the same check we did in check-changelog.yml:

scikit-learn/.github/workflows/check-changelog.yml

Line 26 in 9cfacf1

if [[ ! "$changed_files" =~ tests ]]

I would like to use the milestone

1. to automate backports (this is why I'm pushing to have the check before 0.24.2),
2. to guide contributors to fill the right changelog

Reason 1 makes me prefer not to apply the selection on which files have been modified.

Reason 1 makes me prefer not to apply the selection on which files have been modified.

Ah, are you thinking about the PRs that update the documentation that would be backported?

Thinking about this more, the milestone check may be a worst experience for contributors. The milestone check would make the CI fail and the only way for it to pass is for a maintainer to add it to a milestone. In other words, a contributor can not fix the failing CI.

Reason 1 makes me prefer not to apply the selection on which files have been modified.

Ah, are you thinking about the PRs that update the documentation that would be backported?

Yes, and also other kind of PRs, see the 0.24.2 milestone where changelog is not needed for a number of PRs.

Thinking about this more, the milestone check may be a worst experience for contributors. The milestone check would make the CI fail and the only way for it to pass is for a maintainer to add it to a milestone. In other words, a contributor can not fix the failing CI.

The goal here is to automate things that can be automated and to free time for reviewers to review the pull requests: having pull requests waiting for weeks and months is definitely a worst experience than seeing a red check, especially if we advertise on the list and the wiki.

Thinking about this more, the milestone check may be a worst experience for contributors. The milestone check would make the CI fail and the only way for it to pass is for a maintainer to add it to a milestone. In other words, a contributor can not fix the failing CI.

I think that we have a need for the PR to automatize the release process, especially the bug release. However, @thomasjpfan you are raising similar points that we discuss with @cmarmo IRL. But, I did not find a better framework while discussing.

Something that I did not think (and I am still not sure this is possible), would be to use the branch protection with some "review required" of at least 1 team member as done in scikit-image: scikit-image/scikit-image#5313. If we are able to detect the status "review required", we could bypass the check for the milestone and the changelog. Those checks could be run once the first done with an associated milestone assigned by the reviewer.

I have added a note in the title of the check to clarify that it needs a triage intervention when red.

we could bypass the check for the milestone and the changelog

To be more explicit, we have a set of minimum tests allowing to have the CI green that could be less disturbing to contributor.

I guess this is okay, we already have the "No Changelog Required" tag, which a maintainer needs to add to PRs that do not require a changelog. This is not something a contributor can do.

As for PRs that do not need to be milestoned, we can skip the check if it has the "Build / CI" label.

As for PRs that do not need to be milestoned, we can skip the check if it has the "Build / CI" label.

This would not work 100% of the time, but it should cover most cases. We would need to remember to milestone CI changes that need to be in a bugfix release.

Note that a skip is possible with the "Long Term" label.... that I can't create... @thomasjpfan , @glemaitre , do you mind creating it as we can test the skip before merging?
Thanks!

I added the label

Would it be correct to label CI changes as "Long Term"? I would categorize it as "No Milestone Required".

Would it be correct to label CI changes as "Long Term"? I would categorize it as "No Milestone Required".

I don't see why CI or build pull requests can't be milestoned: for example if some dependency version is planned to be changed in a specific version we can milestone PRs that depends on this change. I believe that starting simple without exceptions is often a good compromise.

As a test, the label in action.

I have been asked to manually check the ChangeLogs in preparation of 0.24.2: I strongly believe that an automatic check like the one proposed in #19596 would avoid human intervention and make the release process more efficient

@cmarmo would you mind describing how this PR will ease the work of checking the changelogs?

Thinking about this more, the milestone check may be a worst experience for contributors. The milestone check would make the CI fail and the only way for it to pass is for a maintainer to add it to a milestone. In other words, a contributor can not fix the failing CI.

I really share that concern. Is every new PR going to fail just because of this new check now? That doesn't sound like something we'd want. We need to be careful about contributor experience, and an uncooperative CI can be a significant source of frustration.

Looking at #19788 and #19731 on which this new check has been running, it doesn't seem like it's working as expected.

FYI milestone events have been added in github workflow management.