-
Notifications
You must be signed in to change notification settings - Fork 100
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
Option to only update when certain required status are passing #259
Comments
This does sound like a reasonable way to start addressing issues with excessive updates while not tracking PR state in Bulldozer - thanks for suggesting it!
I think this makes sense. If I explicitly request updates, on a pull requests, I think it would be confusing if they also had to satisfy other conditions. Also, explicit requests are less likely to cause issues with excessive updates in the first place. While this is different from how |
+1 for this suggestion. Would be really useful to us as we also have a lot of unneeded updates with our PRs. |
+1 on this suggestion too. Currently it updates all PRs when the target branch is updated, triggering a ton of validation jobs unnecessarily as they will need to be executed again later when they are first in the queue. I would like to be able the PR to only be updated once it is the first in the queue and it is going to be merged. On a PR that fails the CI: It should be discarded from the queue, or put back at the end of it. @bluekeyes would that be possible to implement? How hard would be to implement it, so I can see if I can help? |
The original suggestion on this ticket (adding a One clarification: Bulldozer does not maintain a queue of PRs to update. It simply lists the open PRs from GitHub and then updates them in the order they are returned. If you think there is a sort order or other heuristic using data from GitHub that would give better results, I'm also happy to review PRs for that change, but I'd like to avoid adding state (like a persistent queue) to Bulldozer. |
Would it be possible to look at all open PRs and find the PR with the most consecutive update commits from Bulldozer as the top priority for merge? Or perhaps a configurable label matching for priority? I can see how a lack of local state might cause issues and race conditions, though. If Pull Request Merge Queue ends up landing in full support from GitHub, that seems like a better long-term option for merge queueing (defer to the platform). It would help remove the need for update merged from Bulldozer. Once that lands, I may investigate further and open issues for potential support in Bulldozer. |
Created #297 for supporting GitHub's merge queue (once it is stable). I'm planning to work on contributing this feature this week 🤞🏻 |
Deleted my unrelated comment, accidentally posted on the wrong issue 😬 😞 |
Raised a draft PR for this enhancement: #307 See #307 (comment) for one consideration where I could use feedback. Status or check runs not being fulfilled is working, but completion of status or check runs is not triggering updates as currently implemented. I'm thinking it should, but open to feedback before I dig in on that part of this change. |
Similar to how
merge
can be configured to not take action unless allrequired_statuses
are passing or neutral, it would be helpful ifupdate
could be configured to not take action unless allrequired_statuses
are passing or neutral. This would help prevent failing builds from being continuously updated and wasting CI resources.Considerations for implementation:
required_statuses
should be separately configured forupdate
andmerge
required_statuses
forupdate
should be optionalupdate
support forrequired_statuses
should reuse the configuration interfaces and code implementations for the equivalent support inmerge
wherever possible, to help keep things consistent between the two and reduce maintenance over time.Open questions:
trigger
conditions override therequired_statuses
like they do forignore_drafts
(added in Allow ignoring Draft PRs from updates #256)? This would allow users to manually override failing statuses to keep updates happening (in case an update is expected to resolve a failing status)The text was updated successfully, but these errors were encountered: