Skip to content

.github/workflows: add auto label merge conflicts#99064

Closed
ryantm wants to merge 1 commit into
NixOS:masterfrom
ryantm:mc
Closed

.github/workflows: add auto label merge conflicts#99064
ryantm wants to merge 1 commit into
NixOS:masterfrom
ryantm:mc

Conversation

@ryantm

@ryantm ryantm commented Sep 29, 2020

Copy link
Copy Markdown
Member

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.

@ryantm ryantm requested review from Mic92 and zowoq as code owners September 29, 2020 05:34
@ofborg ofborg Bot added 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Sep 29, 2020
@zowoq

zowoq commented Sep 30, 2020

Copy link
Copy Markdown
Contributor

Looks like this checks the 100 most recent PRs so we might want to run this a couple of times a day.

Comment thread .github/workflows/auto-label-merge-conflicts.yml Outdated
@ryantm

ryantm commented Sep 30, 2020

Copy link
Copy Markdown
Member Author

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:
https://github.com/mschilde/auto-label-merge-conflicts/blob/2f64072651aa21e322975df6125041d5421a5864/lib/queries.ts#L78

@zowoq

zowoq commented Sep 30, 2020

Copy link
Copy Markdown
Contributor

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.

@ryantm

ryantm commented Sep 30, 2020

Copy link
Copy Markdown
Member Author

@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.

@zowoq

zowoq commented Oct 1, 2020

Copy link
Copy Markdown
Contributor

https://docs.github.com/en/free-pro-team@latest/actions/reference/usage-limits-billing-and-administration#usage-limits

API requests - You can execute up to 1000 API requests in an hour across all actions within a repository. If exceeded, additional API calls will fail, which might cause jobs to fail.

https://docs.github.com/en/free-pro-team@latest/actions/hosting-your-own-runners/about-self-hosted-runners#usage-limits

Self-hosted seems to have the same limit but it it counted separately from github-hosted.

Looks the 1000 does include self-hosted.


Do we have a way to pay GitHub for more API quota?

No idea, sorry.

@doronbehar

Copy link
Copy Markdown
Contributor

@zowoq I think it will be really useful for people to filter PRs for review, so I consider solving this a pretty high priority.

@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.

@ryantm

ryantm commented Oct 1, 2020

Copy link
Copy Markdown
Member Author

@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.

@doronbehar

Copy link
Copy Markdown
Contributor

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.

@ryantm ryantm force-pushed the mc branch 3 times, most recently from 6307937 to a2530df Compare October 4, 2020 00:07
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.
@ryantm

ryantm commented Oct 4, 2020

Copy link
Copy Markdown
Member Author

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.

@ryantm ryantm requested a review from zowoq October 4, 2020 00:23
@zowoq

zowoq commented Oct 4, 2020

Copy link
Copy Markdown
Contributor

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.

@Mic92

Mic92 commented Oct 4, 2020

Copy link
Copy Markdown
Member

The stale bot ignores bots: https://github.com/probot/stale/blob/e3c53ebf879cccaf856fba1602fdd67f878c3ee3/index.js#L24
so we need to make sure the auto label bot is also marked as such.

@SuperSandro2000

Copy link
Copy Markdown
Member

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.

@SuperSandro2000 SuperSandro2000 marked this pull request as draft November 28, 2020 00:13
@stale

This comment was marked as spam.

@stale stale Bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 3, 2021
@FliegendeWurst FliegendeWurst added the 6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions label Mar 1, 2025
@FliegendeWurst FliegendeWurst removed the 6.topic: policy discussion Discuss policies to work in and around Nixpkgs label Mar 29, 2025
@nixpkgs-ci nixpkgs-ci Bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jun 25, 2025
@zowoq

zowoq commented Jun 25, 2025

Copy link
Copy Markdown
Contributor

Merge conflict labels are now applied by github actions #419654.

@zowoq zowoq closed this Jun 25, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

6.topic: continuous integration Affects continuous integration (CI) in Nixpkgs, including Ofborg and GitHub Actions 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants