feat: add merge group handler in support of github merge queues #554
Conversation
server/handler/merge_group.go
Outdated
| Config: fetchedConfig, | ||
| } | ||
|
|
||
| _, err = evalCtx.ParseConfig(ctx, common.TriggerStatic) |
There was a problem hiding this comment.
might have to make a new method for validating the policy since ParseConfig doesn't return an evaluator for static trigger, so as written this will post a status even if there's no policy file, and i don't think that's what we want
There was a problem hiding this comment.
We might not need to parse the configuration at all. I think it would be enough to only check fetchedConfig to determine if a policy file exists, since the main goal is to not post status checks in repositories that don't enable Policy Bot. If a PR is added to the merge queue, presumably the policy existed and was valid at the time of merge, so I think it could actually be confusing if the policy was changed to be invalid while the PR was in the queue and it led to a failed status.
There was a problem hiding this comment.
good point, sounds good to me, will make that change
There was a problem hiding this comment.
Thanks for adding this, the implementation looks good to me as a starting point. I imagine we might need to make adjustments as people gain experience with merge queues, but this should be a safe way to unblock testing. One minor code issue and then this should be good to merge.
server/handler/merge_group.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if err != nil { | ||
| return errors.Wrap(err, "failed to create evaluation context") | ||
| } |
There was a problem hiding this comment.
Looks like there's a double error check here
Closes #548
Adds support for github merge queues by unconditionally posting a successful status check on the merge group branch when policy bot sees a
merge_groupgithub webhook event.Worked together with @btrautmann to get this done. Let us know if something like this is what you had in mind as a solution for this.