Skip to content
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

CLI command to clean up after a merged PR #380

Open
jglick opened this issue Feb 12, 2020 · 20 comments · May be fixed by #7322
Open

CLI command to clean up after a merged PR #380

jglick opened this issue Feb 12, 2020 · 20 comments · May be fixed by #7322
Labels
core This issue is not accepting PRs from outside contributors enhancement a request to improve CLI gh-pr relating to the gh pr command

Comments

@jglick
Copy link

jglick commented Feb 12, 2020

Describe the feature or problem you’d like to solve

After a PR of mine has been merged I currently have to

  • click the button in the GUI to delete the branch (typically from a fork)
  • navigate to my local clone
  • run:
git checkout master # if not already on base branch
git pull
git branch -d local-branch

This is a bunch of manual steps which I need to repeat several times on a good day.

Proposed solution

Navigate to the local clone and

gh pr cleanup 123

ought to take care of deleting the various branches for me. Or even

gh pr cleanup --all

to clean up branches from any of my PRs which were merged since I last ran this.

Ideally this would fast-forward my local base branch reference without actually needing to check it out, in case I currently had checked out some other (unmerged) branch or had local modifications.

For bonus points, if some rude maintainer used squash or rebase, verify that the base branch really contains all the changes in my local branch before deleting it. Otherwise I need to check this manually if I am being careful. (Genuine merges behave better: if for example I forgot to push some last-minute commits, git branch -d will warn me that the branch is not actually merged. This warning is useless for squashed or rebased PRs because it is always printed.)

Additional context

N/A

@billygriffin billygriffin added pull request enhancement a request to improve CLI needs-design An engineering task needs design to proceed labels Feb 12, 2020
@billygriffin
Copy link
Contributor

Thanks for so clearly laying out the steps you're taking today @jglick, this is really helpful. I marked this as a potential enhancement that we need to think about the UX around some. This isn't one of the next few things we'll get to but it definitely feels like it has the potential to alleviate that pain.

@Jackenmen
Copy link
Contributor

Jackenmen commented Feb 13, 2020

If I'm not owner of the repository and am using fork, I also have to push (well, I don't have to but keeping the fork up-to-date is always good to have) updated base branch after pulling from upstream so would be great to have that available too.

@francois
Copy link

I do the same thing as well, using this script:

$ cat bin/list-stale-branches
#!/bin/bash
set -o errexit -o nounset -o noclobber -o pipefail

echo "These branches are stale and can be deleted:"
echo

git branch --merged | grep --invert-match --extended-regexp '^[*]|master' | cut -c 3-

echo
echo "Run this command to get rid of all of them at once:"
echo -n "git branch --delete --force "
git branch --merged | grep --invert-match --extended-regexp '^[*]|master' | cut -c 3- | tr '\n' ' '
echo

It would be very nice to automate this!

@Jackenmen
Copy link
Contributor

@francois I don't think that works with squash merging though, right?

@francois
Copy link

francois commented Feb 17, 2020 via email

@OliverJAsh
Copy link

Is it worth considering this alongside #373? I.e. can we have one command for "merge and then clean up"?

I wrote a script to do this using hub: https://gist.github.com/OliverJAsh/929c761c8ecbf14d0010634a3f015740#file-git-merge-pr

I also shared some ideas on how the cleanup command could be exposed here:

  1. Delete local/remote head branches: --delete-head-branches
  2. Update (pull) local base branch: --update-local-base-branch
  3. 1 + 2 = --sync-branches

mislav/hub#1483 (comment)

@jglick
Copy link
Author

jglick commented Mar 3, 2020

can we have one command for "merge and then clean up"?

May be useful if you happen to be merging your own PR, though I filed this RFE for the general case that the PR has been merged by someone else.

@DanyC97
Copy link

DanyC97 commented Apr 29, 2020

@jglick while this is a good RFE wouldn't this help?

@jglick
Copy link
Author

jglick commented Apr 29, 2020

@DanyC97 that is the first (manual) step in #380 (comment).

@eXamadeus
Copy link

eXamadeus commented May 7, 2020

@jglick I really like this RFE. It's something I've been really wanting to see from a CLI tool for a while.

I have a similar script to @francois:

git branch --merged origin/master | egrep -v "(?:^\*)|(?:master$)" | xargs git branch -D

but unfortunately, the main repo I work on has mandatory squash rules set. I'm interested in your idea of checking if the branches are the same before deleting because a simple git branch -d ... would fail in my scenario every time.

Question: how to compare local vs merged PR branches

What are your ideas for checking to see if the merged and local PR branches are equivalent? We could do something like git diff local-pr..remote/merge-target and making sure the diff is empty. There are definitely issues with just doing a simple git diff ... though. If the local PR branch was behind the remote PR branch and the PR was squashed or rebased, how would we reconcile that? A diff won't work because there will be a variance (unless all the commits were empty).

What would be cleaner is if GitHub keeps internal track of the "HEAD before the squash/rebase" which could then be used to check against the local PR branch like so:

# where $HASH is the HEAD of the remote PR branch before the squash/rebase/merge
git branch --merged $HASH

...and see if the local PR branch is considered "merged" to that $HASH.

Hmmm...thoughts? 🤔

Suggestion: don't update the local base branch ref

Ideally this would fast-forward my local base branch reference without actually needing to check it out

I feel like updating the users local base branch (master) is too unexpected. I would be surprised if I typed gh pr cleanup --all and come to find my base branch was updated by a cleanup command. Not to mention, what would happen if I had commits on my base branch that aren't in the remote yet?

@jglick
Copy link
Author

jglick commented May 8, 2020

What are your ideas for checking to see if the merged and local PR branches are equivalent?

If you are using squash or rebase, I am not sure how it could be done reliably. You are basically going behind Git’s back.

what would happen if I had commits on my base branch that aren't in the remote yet?

You mean if you are using a non-PR workflow for some changes. In that case updating the base branch would not be a fast-forward so would have to be skipped. Anyway, no strong opinion about whether this implicit git pull would be unexpected; if not included in the tool, I would always do it anyway.

@Jackenmen
Copy link
Contributor

If you are using squash or rebase, I am not sure how it could be done reliably. You are basically going behind Git’s back.

Wouldn't it be possible to just compare refs/pull/NUMBER/head with local branch?
If the merge (or squash/rebase) was done on the PR through GH that should work, I don't think we can do much about manual squash merges (done locally and pushed by maintainer without using GH to do it)

@jglick
Copy link
Author

jglick commented May 8, 2020

Wouldn't it be possible to just compare refs/pull/NUMBER/head with local branch?

Yes I suppose that would suffice in the typical case. (Higher rate of false positives than with true merges, where it is fine if the remote head has been updated past your local ref, for example because a maintainer pushed an additional commit prior to merging.)

@Jackenmen
Copy link
Contributor

the remote head has been updated past your local ref

With that in mind, I am thinking the logic could look like this:

  1. Check if upstream/refs/pull/NUMBER/head and origin/branch_name_for_the_pr have identical refs (they should have identical refs at the time of merge/squash/rebase)
    • if refs aren't identical it could do one of two things (to be determined):
      • finish checking and leave the branch as is
      • check if local branch has identical ref or is behind upstream/refs/pull/NUMBER/head - remove the branch if it is, leave the branch as is if it isn't
    • if refs are identical, continue to 2.
  2. If both of the conditions below are true, remove the branch
    • No unpushed local changes (covers the cases where local branch is X commits ahead of origin)
    • Local and origin have clean history/aren't diverged (covers cases where both local and origin have some commits other doesn't)

@vilmibm
Copy link
Contributor

vilmibm commented Sep 16, 2020

does the cleanup support added to pr merge solve this issue?

@Jackenmen
Copy link
Contributor

does the cleanup support added to pr merge solve this issue?

@vilmibm only a rather small part of it. The conditions for it to cleanup the branches are rather specific:
a) PR needs to be merged from cli (which I don't expect most people to do, although I can't prove that with any data)
b) PR needs to be merged by person that made the branch or at least has access to it (which won't be case for forks). In the latter case, it doesn't solve the cleanup of local branch for the PR author though.

So I would not say that pr merge's cleanup solves this issue. Personally, I would still be left with all the branches on my fork (both local and remote) after other people merge my PRs in upstream, so this functionality is still needed for me.

@jglick
Copy link
Author

jglick commented Feb 1, 2021

Even for the special case that I am merging my own PR, gh pr merge -d does not work well currently due to #1444 & #2860.

@samcoe samcoe added core This issue is not accepting PRs from outside contributors and removed needs-design An engineering task needs design to proceed labels Feb 2, 2022
@jakub-g
Copy link

jakub-g commented Nov 2, 2022

2 cents from me: This would be super useful. I agree with a solution outlined in #380 (comment) by @jack1142:

Let's say

  • jakub/my-awesome-branch locally is abcdef1234
  • jakub/my-awesome-branch was PR 101 on GitHub
  • PR 101 got squash-merged; at the time of the squash, the HEAD of the branch was abcdef1234
  • Great, the shas match -> delete the local branch
  • If shas don't match -> don't delete the local branch, unless --force to delete the branch.

Note: Ideally this should be a separate command like gh branches clean, to clean after things merged 1) via UI, 2) by someone else, 3) via a solution like merge queue.

@0xdevalias
Copy link

0xdevalias commented Dec 7, 2022

Just wanted to chime in my +1 for this being super useful and something I would love for the gh CLI to include; and to also contribute a few alternative projects that might be useful in the meantime:

And from my git aliases (these are fairly old, cobbled together from StackOverflow/etc, might not be the most efficient way to achieve things, but there might be some bits that help solve things here, as I know I have used it for both merged and squash merged):

  # Force delete {local,remote} branch
  force-delete-local-branch         = branch -D

  # Ref: https://stackoverflow.com/questions/39220870/in-git-list-names-of-branches-with-unpushed-commits/48180899#48180899
  list-unpushed                     = list-unpushed-to-origin
  list-unpushed-to-origin           = log --branches --not --remotes=origin --no-walk --decorate --oneline

  # List {local,remote} branches that have been merged into BRANCH
  list-local-merged                 = branch --merged
  list-remote-merged                = branch --remote --merged

  # (Ref: https://stackoverflow.com/questions/43489303/how-can-i-delete-all-git-branches-which-have-been-squash-and-merge-via-github/56026209#56026209)

  # List {local,remote} branches that have been merged into {master,staging}
  list-local-merged-master          = !git list-local-merged master   | grep -v master | grep -v staging
  list-local-merged-staging         = !git list-local-merged staging  | grep -v master | grep -v staging
  list-remote-merged-master         = !git list-remote-merged master  | grep -v master | grep -v staging
  list-remote-merged-staging        = !git list-remote-merged staging | grep -v master | grep -v staging

  # List local branches that have been squash merged into {master,staging}
  list-local-squash-merged-master   = !$ZSH/bin/git-list-local-squash-merged-master
  list-local-squash-merged-staging  = !$ZSH/bin/git-list-local-squash-merged-staging

  # Count {local,remote} branches that have been merged into {master,staging}
  count-local-merged-master         = !git list-local-merged-master   | wc -l
  count-local-merged-staging        = !git list-local-merged-staging  | wc -l
  count-remote-merged-master        = !git list-remote-merged-master  | wc -l
  count-remote-merged-staging       = !git list-remote-merged-staging | wc -l

  # Count local branches that have been squash merged into {master,staging}
  count-local-squash-merged-master   = !$ZSH/bin/git-list-local-squash-merged-master  | wc -l
  count-local-squash-merged-staging  = !$ZSH/bin/git-list-local-squash-merged-staging | wc -l

  # Cleanup {local,remote} merged branches
  cleanup-local-merged-master       = !git list-local-merged-master   | xargs echo RUN THE FOLLOWING IF YOU ARE CERTAIN: git delete-local-branch ;:
  cleanup-local-merged-staging      = !git list-local-merged-staging  | xargs echo RUN THE FOLLOWING IF YOU ARE CERTAIN: git delete-local-branch ;:
  cleanup-remote-merged-master      = !git list-remote-merged-master  | xargs echo RUN THE FOLLOWING IF YOU ARE CERTAIN: git delete-remote-origin-branch ;:
  cleanup-remote-merged-staging     = !git list-remote-merged-staging | xargs echo RUN THE FOLLOWING IF YOU ARE CERTAIN: git delete-remote-origin-branch ;:

  # Cleanup local branches that have been squash merged into {master,staging}
  cleanup-local-squash-merged-master   = !git list-local-squash-merged-master  | xargs echo RUN THE FOLLOWING IF YOU ARE CERTAIN: git force-delete-local-branch ;:
  cleanup-local-squash-merged-staging  = !git list-local-squash-merged-staging | xargs echo RUN THE FOLLOWING IF YOU ARE CERTAIN: git force-delete-local-branch ;:

git-list-local-squash-merged-master:

#!/bin/sh
#
# List all local branches that have been merged into master.
#
# Ref: https://stackoverflow.com/questions/43489303/how-can-i-delete-all-git-branches-which-have-been-squash-and-merge-via-github/56026209#56026209

# TODO: refactor this to be able to pass in the branch name via props and have it 'just work'
git checkout -q master && git for-each-ref refs/heads/ "--format=%(refname:short)" | while read branch; do mergeBase=$(git merge-base master $branch) && [[ $(git cherry master $(git commit-tree $(git rev-parse $branch^{tree}) -p $mergeBase -m _)) == "-"* ]] && echo "$branch"; done

git-list-local-squash-merged-staging:

#!/bin/sh
#
# List all local branches that have been merged into staging.
#
# Ref: https://stackoverflow.com/questions/43489303/how-can-i-delete-all-git-branches-which-have-been-squash-and-merge-via-github/56026209#56026209

# TODO: refactor this to be able to pass in the branch name via props and have it 'just work'
git checkout -q staging && git for-each-ref refs/heads/ "--format=%(refname:short)" | while read branch; do mergeBase=$(git merge-base staging $branch) && [[ $(git cherry staging $(git commit-tree $(git rev-parse $branch^{tree}) -p $mergeBase -m _)) == "-"* ]] && echo "$branch"; done

@elldritch
Copy link

I've thrown together a draft of this in #7322, if anyone wants to try it out (clone the branch and make). If anyone has the bandwidth, it would be great to get some help with tests.

@mislav mislav removed their assignment Jun 9, 2023
@samcoe samcoe added the gh-pr relating to the gh pr command label Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core This issue is not accepting PRs from outside contributors enhancement a request to improve CLI gh-pr relating to the gh pr command
Projects
None yet
Development

Successfully merging a pull request may close this issue.