[CI] Add a workflow to check ready PRs #19938
Conversation
This workflow checks for ready-to-merge PRs - if a PR is open, not a draft, passed all checks, and has been approved, it will ping @intel/llvm-gatekeepers if this group has not already been mentioned or if the last mention was more than $days days ago.
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
| days=3 | ||
| days_in_seconds=$((days*24*60*60)) | ||
|
|
||
| # Function to ping gatekeepers and print debug info |
There was a problem hiding this comment.
Looks like the function doesn't print the debug info. Should it be removed from the comment?
There was a problem hiding this comment.
This is some kind of debug info, isn't it?:D
echo "Pinged @intel/llvm-gatekeepers for https://github.com/intel/llvm/pull/$pr_number"
There was a problem hiding this comment.
Ah, if this is a debug info, then okay;) It looked like logging for me
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
thx! ready for merge? |
|
@sarnex yep |
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
KornevNikita
temporarily deployed
to
WindowsCILock
GitHub Actions
Inactive
|
|
||
| on: | ||
| schedule: | ||
| - cron: '0 * * * *' # every hour |
There was a problem hiding this comment.
Once a day seems more reasonable to me.
What is the reason to run this workflow every hour?
There was a problem hiding this comment.
the workflow runs once an hour but it only adds a comment if there has been no ping in 3 days
There was a problem hiding this comment.
@bader the more often we launch this workflow, the faster gatekeepers get a notification about ready-to-merge PRs. But if you think it's too often, we can switch to a daily basis. Should we?
There was a problem hiding this comment.
I don't know. The PR description doesn't have problem statement. Are you trying to merge PRs faster? How fast is good enough?
Why not to make it immediate? You need to make all the checks you mentioned in the PR description at two events:
- PR is approved.
- Pre-commit testing is finished.
Another alternative: enable auto-merge for "ready-to-merge" PRs. Is there any reason for gatekeepers to not merge such PRs?
There was a problem hiding this comment.
Another alternative: enable auto-merge for "ready-to-merge" PRs. Is there any reason for gatekeepers to not merge such PRs?
I also thought of this, we do this for the internal fork already. I couldn't find out how to do it in the GitHub settings but IMO this is the best long-time thing to do. We probably need to get a sign off from stewart though
There was a problem hiding this comment.
The check verifying that author's e-mail is publicly visible doesn't work properly, so we have to have humans in the loop at least for that.
There was a problem hiding this comment.
FYI: PRs are approved by humans.
There was a problem hiding this comment.
As I understand an auto-merge should be enabled for every PR manually, so it's not possible to auto-merge all ready-to-merge PRs - if that's what you're talking about.
There was a problem hiding this comment.
What we have internally is a bit different, the PR isn't auto merged but all developers are able to merge the PR themselves if all requirements are met
There was a problem hiding this comment.
As I understand an auto-merge should be enabled for every PR manually
I'm 99.(9)% sure it can be scripted.
Merging by developers with write access to the repository sounds good to me too.
5 participants
This workflow checks for ready-to-merge PRs - if a PR is open, not a draft, passed all checks, and has been approved, it will ping @intel/llvm-gatekeepers if this group has not already been mentioned or if the last mention was more than $days days ago.