batches: automatically delete branches when changeset is merged/closed with site config setting#52055
Conversation
| CommitID string `json:"commitId"` | ||
| MergeStrategy *PullRequestMergeStrategy `json:"mergeStrategy"` |
There was a problem hiding this comment.
The removal of the json tags here is intentional; this type isn't directly used as any request payload, it's only the input type of the CompletePullRequest method, so the tags are unnecessary as an object of this type is never encoded as JSON.
| if resp.StatusCode != http.StatusNoContent { | ||
| err = json.NewDecoder(resp.Body).Decode(result) | ||
| } |
There was a problem hiding this comment.
We would see errors reported whenever the new DeleteBranch method was invoked for GitHub because the response is a 204 No Content, so the body would be totally empty and the JSON decoding would freak out when it immediately read in EOF. They were harmless errors, but this just cleans that up.
This is also covered by the GitHub integration test I added.
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 929cc1e...836141f.
|
BolajiOlajide
left a comment
There was a problem hiding this comment.
This looks good to me.
I haven't tested with all code hosts yet (only GitHub & GitLab), but I plan to get around that later today.
|
Thanks! Don't feel like you have to test them all right away, either. I want to get the setting enabled on S2 and pay extra attention to it between now and when we cut the patch release. :D |
Alright. Let's test further on S2. Excited for this. |
…d with site config setting (#52055) This adds a new site setting, batchChanges.autoDeleteBranch, which, when enabled, will have Sourcegraph attempt to delete the branches for any changesets it has created when the changesets are merged or closed. - If a code host supports this natively with a flag on the changeset, great! At any point when we would create or update the properties of a changeset on the code host, we check the setting in site config and make sure to send its updated value along in the payload, ensuring the flag always stays closely synced with the setting on Sourcegraph. - If a code host does not support this natively, at any point when we close or merge a changeset on the code host, we check the setting in site config and, on successful close/merge, we send a follow-up request to delete the branch associated with the changeset.
Closes https://github.com/sourcegraph/sourcegraph/issues/50354.
This adds a new site setting,
batchChanges.autoDeleteBranch, which, when enabled, will have Sourcegraph attempt to delete the branches for any changesets it has created when the changesets are merged or closed.Ideally, this functionality would be opt-in per batch change or even per changeset operation, so there's room for future iteration here. This PR merely provides the groundwork to enable this functionality and exposes it via a global site config setting so that customers can start taking advantage of it immediately.
Code hosts offer varying levels of support for this. Several provide a 1st-party solution to this via a property on the changeset itself that can be toggled. But a few don't. For those, Sourcegraph must instead issue a secondary API call to delete the branch after it closes or merges the changeset. This does mean that on those code hosts, if the changeset is merged or closed on the code host itself, we won't have any control over whether or not the branch is deleted. I've tried to make this explicitly clear in the docs.
We introduce this logic at the Batch Changes
Sourcelayer, where we have consolidated other logic about the ways Batch Changes should interact with code host API methods. Generally speaking, the approach takes one of the following forms:Test plan
Wherever we already had VCR integration tests set up for Batch Changes
Sources, I wrote additional tests to cover toggling the flag or the additional API calls. Two code hosts don't have any VCR test set ups, so for those I only manually tested with the code hosts. I'd love to add VCR testing for them but just don't have the time right now.