scala-steward icon indicating copy to clipboard operation
scala-steward copied to clipboard

Delete remote branches for merged pull requests

Open stevedlawrence opened this issue 3 years ago • 2 comments

According to https://github.com/scala-steward-org/scala-steward/issues/1793 and https://github.com/scala-steward-org/scala-steward/pull/1841, scala-steward will only delete branches when PR's are closed, and not when they are merged. This means that adding scala-steward as a remote and fetching will list a lot of branches that have potentially already been merged. And the daffodil project uses the "rebase and merge" option, so all these branches make it difficult to see which ones have or haven't been merged. We usually just delete remote branches after they've been merged, so if a branch exists, then it's considered still active.

So, would it be possible to have scala-steward deleted remote branches once pull requests have been merged and the PR closed?

stevedlawrence avatar Apr 07 '21 15:04 stevedlawrence

We use GitLab in our organization, and I managed to add support for automatic branch deletion on merge. It is a GitLab feature, so it is sufficient to just enable it when creating merge request or when enabling automatic merge.

If you don't mind, here is the patch; it is quite simple as we did not need any flag to switch it on or off (removing the branch after merge seems to be a good default behaviour anyway).

gitlab-remove-source-branch.patch.zip

honzabazant avatar Jul 02 '22 10:07 honzabazant

Leaving a note for myself as much as for anyone else who wants to take a crack at this:

In NurtureAlg, forgeApiAlg.listPullRequests sensibly constrains PR lookup to the exact branch for the update that's currently being processed, instead of attempting to filter from the massive list.

This means that once an update is applied, the update will no longer be in the list, which means it's no longer under the purview of scala-steward.

I propose a housekeeping phase:

  1. Paginate all PRs opened by scala-steward which are closed, but whose branches still exist. This can be done via GraphQL, though scala-steward will have to do work to make the GraphQL calls as there are no convenient filters I can find that accomplish this.
  2. When there are is one full page worth of PRs who are both closed and have no branch, begin to process the previous page. This ensures that on each run we make progress on the backlog of stale branches without blocking scala-steward for too long. The presumption here is that the actual deletion of branches is heavy enough that this is a reasonable strategy. If it turns out that the GraphQL queries themselves are the most expensive operation then we should probably just bite the bullet and have a blocking cleanup operation.
  3. A rate limiter can be used to run the clean-up phase at a small random chance, reducing the expense of GraphQL queries as well as processing time in scala-steward.

A sample GraphQL query I came up with that accomplishes step 1 above is as follows:

query {
  search(query: "repo:guardrail-dev/guardrail is:closed is:pr is:public archived:false author:scala-steward", type: ISSUE, first: 100) {
    edges {
      node {
        ... on PullRequest {
          mergedAt
          headRef {
            name
          }
        }
      }
    }
  }
}

An example of a PR that was closed, not merged:

        {
          "node": {
            "mergedAt": null,
            "headRef": null
          }
        },

An example of a PR that was merged, but whose branch still exists:

        {
          "node": {
            "mergedAt": "2023-10-18T22:06:20Z",
            "headRef": {
              "name": "update/scalameta-4.8.11"
            }
          }
        },

Once branches start being deleted, mergedAt will be non-null but headRef will be null. That is the indicator of no further work to be done to resolve this.

blast-hardcheese avatar Oct 19 '23 05:10 blast-hardcheese