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
.github/workflows: add auto label merge conflicts #99064
base: master
Are you sure you want to change the base?
Conversation
Looks like this checks the 100 most recent PRs so we might want to run this a couple of times a day. |
@zowoq I'm pretty sure it tries to fetch all the PRs, not just the most recent ones: |
Oops, TBH I stopped looking once I saw 100 as I assumed it would have a limit. If this is going to check all 2000+ PRs it might use up a bit of the API quota. |
@zowoq I think it will be really useful for people to filter PRs for review, so I consider solving this a pretty high priority. Do we have a way to pay GitHub for more API quota? I'm personall willing to pay the costs to see how much it costs, and if it isn't crazy continue supporting it. Maybe it is also possible to run it on my personal server, so I'll look into that as a backup option. |
Looks the 1000 does include self-hosted.
No idea, sorry. |
@ryantm is the intention of this change to make reviewers know that there is a merge conflict, or that ther isn't? :) TBH I don't see how ofborg's labeling merge conflicts is not good enough. I'm not strictly against the idea, but considering our API quota, this will not use up ofborg's quota? I hope it won't. However, I would have been delighted if some bot would have commented on PRs that there is a merge conflict, and hence notify subscribers of the issue, not only silently label there's a merge conflict. |
@doronbehar Yes, that's the intention. Mainly so reviewers can filter out PRs that are in conflict when reviewing. OfBorg is not good enough because it doesn't update the labels when changes to master introduce the conflict. I think it would affect OfBorg's quota since they would both be workflows on the same project. That's a good point that it would be much more useful if it notifies people. |
Investigating a bit led me to: mschilde/auto-label-merge-conflicts#24 which points to this fork: https://github.com/Marr11317/ConflictAdviser . Note the Limitations. |
6307937
to
a2530df
Compare
OfBorg labels merge conflicts when people push to their branch, but it doesn't relabel PRs when updates to master introduce the conflict. The reason to have good merge conflict labeling is then you can search and filter PRs. GitHub does not provide this feature itself, unfortunately. This took 1hr to run on nixpkgs, but it might be quicker depending on how many merges to master are happening while it runs.
I figured out how to run this locally with act, fixed a bugs with it, and ran it on nixpkgs. It turns out about 1/3 of our open PRs are conflicted! It took about 57 minutes to run, a large part of that time was waiting for GitHub to have a merge status for each PR. Probably it will run faster during times when not many merges to master are happening. I misunderstood earlier when @zowoq mentioned a quota. I thought you were talking about GitHub Actions quota, but it looks like GitHub Actions is free for public repositories. https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-billing-and-payments-on-github/about-billing-for-github-actions If there is a problem with the API quota, it seems like we can slow down how often it makes requests. On my fork, I've already slowed down the merge status requests to happen once every minute instead of once every 5 seconds, and increased the retries from 5 to 60. |
How many API calls per PR and how many PRs an hour? Also this seems to cause the stale label to be removed when it adds the conflict label? Kind of makes the stale bot less useful for PRs. Edit: I guess it would also be a race between the conflict and stale labels as the conflict label will reset the stale countdown. |
The stale bot ignores bots: https://github.com/probot/stale/blob/e3c53ebf879cccaf856fba1602fdd67f878c3ee3/index.js#L24 |
For me this is very important because PRs with merge conflicts can't run with nixpkgs-review and realizing this after walking away is always a bummer. |
I marked this as stale due to inactivity. → More info |
OfBorg labels merge conflicts when people push to their branch, but it
doesn't relabel PRs when there are updates to master. This would do
that.
The reason to have good merge conflict labeling is then you can search
and filter PRs. GitHub does not provide this feature itself,
unfortunately.
I do not know how technically and/or economically efficient this
action is.
I looked over the code for the action, it seems legit.