-
-
Notifications
You must be signed in to change notification settings - Fork 344
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
CI: Assign milestone on merged PRs #4414
Conversation
Clean up
Thanks for your efforts, @echoix . (in my original request, I was thinking of graying out the "merge" button until a milestone is manually assigned) |
Ah, that's another way, but I didn't understand it was that that you meant. (It has the disadvantage that we would be waiting for the GitHub Action queue for every PR, unless we don't set it as required, and it won't help at all). At least with this PR, PRs can stay a long time before getting merged. |
Can someone take a look at this? It is what I think is the most appropriate solution, covering most cases I could think of, that allows us to never need to set milestones on PRs, while still allowing us to set them manually when we want to. It is in essence quite simple, but is a bit longer in order to really « show its work » in the steps outputs, so it isn’t a black box. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm hopeful that this does what's promised in the description.
But also, I don't like how complex it is. The required milestone approached by @neteler seems like simpler and more aligned with what we do for titles.
The problem with that is that in cases where the CI has a long queue, it will be slowing us down, as we would be waiting on it. And it still requires the work of always adding the milestone, instead of just needing it when the current milestone isn't the correct one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let's try that.
(Let the action set the milestone, once it gets its turn in the queue) |
Works as tested and expected |
@echoix will you backport this PR to G8.4? |
I don't really know how the backports work, it wasn't made yet? |
Note that this only gets triggered for pull requests that are closed and merged, so won't work if you do the backports by directly committing to the branch (a commit doesn't have a milestone to check if it is already there) |
In the release branch, you |
* CI: Create a workflow to assign milestones * Add GH_TOKEN for gh cli * CI: Add GH_REPO for gh cli * CI: View PR from gh cli using html_url * CI: Download and parse version file * CI: Show version file * CI: Download and parse version file using a pipe * Show version file output Clean up * Pipe version file env to head * Rename variables to milestone and title * Edit PR with milestone * Get milestone from gh cli * Add comment on why API call is used for getting milestone * Show if the PR has a milestone set or not * Do not run steps if a milestone is already set * Add pull_request_target closed trigger * Add ref as url parameter * Remove debugging steps * CI: Handle RC followed by numbers in sed pattern replacement
I think I did it, can you check if it is right? |
Yes, all fine. |
Addresses #4398 (comment)
This PR assigns a milestone when a PR is closed if none is set, using
pull_request_target
trigger withclosed
activity type. It uses up an API call to know if the PR has a milestone set (limit is 5000 per hour when authenticated). If I used the info from the github event context, the information is populated when the job is queued, and will maybe not represent the state once it will run, and will keep the old state if a rerun is made. I tried to use the gh cli instead of their javascript action's ecosystem so steps could be reproduced locally too using the same gh cli calls.If there are no milestones for that PR, then it uses a second API call to fetch the include/VERSION file at the ref of the merged result (so pull_request_target doesn't matter here, it is still up-to-date). It parses that file like for the coverity scan workflow, but also removes "dev" and "RC" followed by 0 or more numbers. This builds the milestone title to apply, using a third API call.
Using the VERSION file allows it to keep working on newer and older versions (working on older branches if the workflow is backported)
Known limitations: