-
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
Can bulldozer wait until non-required checks are finished before merging? #311
Comments
Bulldozer will only wait for statuses that it knows about up front. This is because in general, there is no way to know when looking at a PR if all the expected status are already present or if more statuses would appear if we waited a bit longer. There are two ways to tell Bulldozer about statuses:
There's currently no way to make Bulldozer wait for failed statuses (both of the methods above require success) or make Bulldozer wait for a status without explicitly listing it. |
Thanks for the information. Hard-coding statuses by their name in
I'm quite sure that bulldozer only needs to wait a few milliseconds before all statuses are available to be evaluated. Can a EDIT: added another bit of rationale for why |
I wonder if Bulldozer could have an additional option to wait for all statuses or checks that are present to complete before merging? |
For any particular status, it should only need to appear in one place: either the GitHub protected branch configuration (if it applies to both humans and to Bulldozer) or the Regarding waiting for either success or failure, I don't fully understand the use case (I'm having trouble thinking of checks where I'd want it to finish but then not care about the result), but would be fine adding an option for this if you or someone else would like to contribute it. On the bigger topic of automatically waiting for statuses, I'm still pretty hesitant. An important design goal for Bulldozer is that (once configured) it should not merge a pull request unexpectedly. When something goes wrong, in Bulldozer or in GitHub, I'd rather leave a pull request open and have someone manually merge or trigger Bulldozer than have to revert a PR that wasn't ready. I haven't yet thought of or seen a suggestion for how to implement some kind of automatic waiting that satisfies this goal. Part of this is just how statuses work in GitHub: they don't exist until something with write permission makes the API call to create one. That means there's no universal pattern that status creators have to follow and leaves a bunch of places for failure (I've seen all of these actually happen):
As a specific example of the last point, CircleCI allows users to create a graph of jobs that execute on each new commit. Circle only posts the pending status for a job when all dependencies complete. Unless I know something about the workflow upfront, when Bulldozer is evaluating a new success status, it doesn't really know if that's the last job or if another one is going to start. In any particular case, you could probably pick delays or a heuristic that works most of the time but I'm not sure it would generalize and I'm still reluctant to have any guessing involved in the merge decision. When something does go wrong with CI or the approval app or whatever, I'd like to not make it worse by adding a bunch of incorrectly merged PRs that need reverts. |
These are non-required checks that are added but don't require success (from a human's perspective) to be merged. Plenty of bots and even humans add checks that are just informational, see Snyk, Chromatic, etc. Off hand, I know Chromatic automatically adds a check that builds a Storybook at a URL accessible after the PR has been merged. It can fail. But we don't need that check to be required for a human or bot to merge. Just to be clear, I'm not saying this feature shouldn't wait until all checks are successful. I'd actually be okay with bulldozer not automatically merging unless all checks successful. But I'd like for it to at least wait for the checks to complete before merging, which I think is a common expectation. Regarding automatically waiting for statuses, I understand the hesitancy around the delay option. |
|
The unsatisfying answer is that I don't know how to do this while maintaining what I believe are the requirements for Bulldozer (specifically, don't merge unexpectedly, even when other systems fail.) If you or anyone else has suggestions or knows of specific open source projects I could look at for reference, I'm interested - I've thought about this for a bit and haven't had any good ideas yet. For instance, if you're talking about the In your second example, I believe that |
Could the requirements here be simplified to "if configured, do not merge a PR while it has any statuses or checks that are not complete?" |
Hello! Love bulldozer but having some trouble understanding something: Bulldozer seems to not wait for non-required checks to finish before merging. But the docs says that bulldozer will:
Here's an example of bulldozer merging a PR without waiting until the "UI Tests" check completes.
How do we get bulldozer to wait until all non-required checks to complete (regardless of a failure or success) before merging?
The text was updated successfully, but these errors were encountered: