Remove merged branches #159

tkirill · 2014-10-02T13:34:24Z

Add the cmdlets for removing merged branches:

Remove-MergedLocalGitBranches -- removes local merged branches.
Remove-MergedRemoteGitBranches -- removes remote merged branches.
Remove-MergedGitBranches -- removes both local and remote merged branches.

Implementation placed in the separate file GitRemoveMergedBranches.ps1 although most of other exported cmdlets sit in GitUtils.ps1. Hope it's not a code style rule to place everything inside GitUtils.ps1.

This is how these cmdlets work.

Remove-MergedLocalGitBranches

Call git branch --merged.
For each line trim whitespaces from both sides.
Filter out line starting with "*" -- it is the current branch.
Filter out ignored branches -- master and develop by default.
If no branch remains then print message and return.
For each remaining branch call git branch -d $item.

Remove-MergedRemoteGitBranches

Works same as Remove-MergedLocalGitBranches except three things:

List of merged branches is retrieved by git branch -r --merged.
Slightly more complex parsing and filtration. We need to filter out the line with "/HEAD ->" and cut off <remote-name>/ part from a beginning of each line.
Removes branch with git push origin :$item.

Remove-MergedGitBranches
This one simply calls previous two cmdlets.

All three cmdlets support -WhatIf and -Confirm (link1, link2). Also, they all support -Ignore parameter for overriding list of ignored branches.

I haven't tested it enough yet to my shame but I think it is a good idea to create PR to receive feedback as soon as possible.

fixes #79

Verb 'Remove' was used instead of 'Delete' because it is approved verb: http://msdn.microsoft.com/en-us/library/ms714428%28v=vs.85%29.aspx. There are three cmdlets: * Remove-MergedLocalGitBranches -- removes local merged branches. * Remove-MergedRemoteGitBranches -- removes remote merged branches. * Remove-MergedGitBranches -- removes both local and remote merged branches. This is draft so there are no settings like target branch or remote repository name or list of branches to ignore.

Default list is master and develop.

dahlbyk · 2014-11-16T03:50:48Z

Terribly sorry for the delay in getting to a review of this (:new: :baby:) - this is a great start! Specific feedback incoming...

dahlbyk · 2014-11-16T03:55:32Z

GitRemoveMergedBranches.ps1

+    !$branch.StartsWith("*")
+}
+
+function TrimRemote-GitBranch($branch)


Maybe switch this to a filter so you can replace ... | foreach {TrimRemote-GitBranch $_} | ... with ... | TrimRemote-GitBranch | ...?

Yes, it looks better now (e093459).

dahlbyk · 2014-11-16T04:34:05Z

General question: would it be useful for any/all of these to accept a parameter (defaulting to HEAD) to identify the ref to check mergedness against, e.g. Remove-MergedGitBranches master?

…ergedBranchesToKeep.

tkirill · 2014-11-19T15:52:20Z

General question: would it be useful for any/all of these to accept a parameter (defaulting to HEAD) to identify the ref to check mergedness against, e.g. Remove-MergedGitBranches master?

I have no strong opinion on this. As for me, it is unlikely I will ever need this parameter.

tkirill · 2015-05-14T06:27:13Z

I found a bug. Remove-MergedBranches works bad with multiple remotes because it doesn't respect remote name in the output of git branch --remote --merged. For example:

C:\Users\t_kirill\workplace\gofra\repo [develop]> git branch --remote --merged
gitlab/counter-page-G-1202
gitlab/develop
gitlab/direct-link-excel
origin/HEAD -> origin/develop
origin/develop

If we have two remotes then merged branches will be deleted in the origin only. Remove-MergedBranches sees merged branches in remotes other than origin but can't delete them:

Deleting remote branch counter-page-G-1202...
error: unable to delete 'counter-page-G-1202': remote ref does not exist
error: failed to push some refs to 'ssh://git@stash:7999/g/gofra.git'

I'll fix it on this week.

dahlbyk · 2015-05-14T07:08:41Z

GitRemoveMergedBranches.ps1

+    }
+}
+
+function Get-MergedLocalGitBranches($ignore)


Drop unused $ignore?

$ignore was completely removed in 47e6c16.

dahlbyk · 2015-05-14T07:19:06Z

Revisions from six months ago (ugh) look great. Looking forward to the fix for multiple remotes!

Would be simpler/cleaner to export a single Remove-MergedGitBranches that accepts a -Remote parameter to switch from local deletes to remote (a la git branch -r)?

I haven't used it in the last 6 month. This functionality is likely redundant.

"Remote" switches between remote and local branches like `git branch --remote`. With this switch we don't need to export `Remove-MergedLocalGitBranches` and `Remove-MergedRemoteGitBranches` so we export only `Remove-MergedGitBranches` now. "All" switches between both remote and local branches and local only.

Default value is "origin". Also, it is not required to pass PSBoundParameters. Is works automatically.

tkirill · 2015-05-22T13:24:55Z

@dahlbyk Your idea about -Remote parameter is good! I implemented it in 31ce862 and it is really better now. Also, I added -All parameter so it is possible to delete both merged and local branches in one call Remove-MergedGitBranches -All.

Bug with multiple remotes fixed in 98956d1. Remove-MergedGitBranches works with origin by default now. There is -RemoteName parameter to switch to another remote. I think an ability to work with multiple remotes in one call by passing an array to -RemoteName is redundant. What do you think?

I've tested different combinations of parameters but not in depth. I used three repositories:

origin
clone 1 of origin
clone 2 of origin with clone 1 as additional remote

I used this script to create this test rig: https://gist.github.com/tkirill/6dd2606d1ae8b9b1e0f5.

Also, I reformatted GitRemoveMergedBranches.ps1 in 7cf9803 basing on the coding style of this project.

Conflicts: GitPrompt.ps1

tkirill · 2015-05-22T14:15:06Z

GitRemoveMergedBranches.ps1

+    foreach ($item in $branches) {
+        if ($PSCmdlet.ShouldProcess($item, "delete remote branch")) {
+            Write-Host "Deleting remote branch $item..."
+            git push $RemoteName :$item


No whitespace escaping for $RemoteName but I think it is safe since it is not possible to use whitespaces in remote's name in Git. @dahlbyk What do you think?

Correct, no need to handle whitespace in remote name.

dahlbyk · 2015-05-23T05:29:52Z

-All is a welcome addition, good call.

On the -RemoteName front, it seems like switching to string[] would be pretty simple implementation-wise, without changing the single-remote usage at all. I wonder if a more reasonable default behavior would be to remove all remotes' merged branches (I.e. not filter by remote name)?

Maybe someone more in tune with the Zen of PowerShell can chime in, but it feels like most of the Write-Host here may be better as Write-Verbose? Not sure what the correct way is to implement an actual -Verbose flag, tho.

tkirill · 2015-05-23T15:29:39Z

@dahlbyk I added support for multiple remotes in 1b881b5. This is really convenient, good that you noticed this!

I wonder if a more reasonable default behavior would be to remove all remotes' merged branches

The only potential problem I can think of is read-only remote. If I fork some project on GitHub I have two remotes:

origin -- my fork. I have write access.
upstream -- original repository. I need this remote for synchronization. I don't have write access.

I think it is a frequent case and in this case working with all remotes by default doesn't work well. However, we definitely need ability to work with all remotes if we want. I thought to add flag -AllRemotes but we already have -All and these two look confusing together. What if instead of working with all remotes by default we implement these flags:

-Remote. Delete only remote branches in origin.
-Local. Delete only local branches.
-RemoteName. Set an array of remotes' names.
-AllRemote. Delete remote branches in all remotes.

Flags -Local -AllRemote delete all possible merged branches. Default behavior without any flags equals to -Local -Remote. It used to work like this before I added -Remote flag yesterday and it was convenient for me. What do you think?

most of the Write-Host here may be better as Write-Verbose

Surprisingly -Verbose already supported by PSCmdlet.ShouldProcess! So I removed Write-Host that duplicated PSCmdlet.ShouldProcess in 0d42639. Result:

Without -Verbose:

With -Verbose:

Default behavior is to delete remote branches in origin only.

dahlbyk · 2015-05-23T18:24:17Z

Remote operations being more "destructive" than their local counterparts, how about we remove any sort of default remote behavior?

Delete-MergedGitBranches = Delete-MergedGitBranches -Local = delete merged local branches.
Delete-MergedGitBranches -Remote= error, not enough information
Delete-MergedGitBranches -Remote origin= delete merged remote branches on origin
Delete-MergedGitBranches -Local -Remote (git remote) = delete all merged local and remote branches

tkirill · 2015-05-24T09:20:11Z

@dahlbyk I like this idea! 👍 We have only two flags -Local and -Remote and they allow us to do everything we want. I'll implement this today or tomorrow.

To be honest I don't find removing merged branches in origin dangerous and every time I delete local branches I want to delete them in origin as well. So Remove-MergedGitBranches == Remove-MergedGitBranches -Local -Remote origin would be more convenient for me but I can create alias 👌

P.S. You wrote Delete-MergedGitBranches instead of Remove-MergedGitBranches. Not sure if it is mistype or not :)

dahlbyk · 2015-05-24T21:01:30Z

Yes, mistyped. Remove is cool.

Dangerousness likely depends on team size and whether or not you use forks - I've dealt with work people doing a "matching" git push --force, so I'm pretty tentative about bulk operations against remotes.

Now there is no default set of remotes. You have to specify list of remotes manually. There is `-Local` switch. If you want to remove only remote branches you can pass only `-Remote string[]`. If you want to remove local branches too you can pass both parameters: `-Local -Remote string[]`.

tkirill · 2015-05-27T07:55:11Z

@dahlbyk Take a look at new arguments please 27e75e5.

Surprisingly validation for empty set of remotes' names works automatically. I didn't found this in documentation.

This is how it looks

I wrote some tests and this test finds another bug with remote branches:

Create repository with one commit to master.
Create another branch branch1. Do not merge it.
Checkout master again.
Clone repository.
Go to clone.
Checkout branch branch1.
Checkout master and merge branch1.
Call Remove-MergedGitBranches -Remote origin.

It will remove origin/branch1 since this branch merged in local master.

I tried to fix this bug in 0f51d0d by checking remote branches against remote-tracking branch corresponding to current branch but this is not enough for the case of multiple remotes.

So I change the way we work with remote branches (again 😕). How it works now:

-Remote parameter is replaced by -RemoteBranch parameter. This parameter holds target branches. Not sure -RemoteBranch is a good name. Maybe -RemoteTarget or -Remote are better?
For each target branch we search for merged branches from the same remote. Doing so totally separates remotes from each other and from local repository.

Working with remotes is more difficult than I thought 😕 Maybe I just don't see some simple way... What do you think about this new idea?

…edGitBranches. If branch merged in remote repository but not merged locally we should remove only remote branch. After you remove remote branches you should pull changes from remote so the branch becomes merged locally.

Now cmdlet requests target branch in `-RemoteBranch` parameter. It checks merged branches against target in the same remote as target branch.

tkirill · 2015-05-31T14:47:57Z

Now after few days I doubt the new way is better. Your suggestion about Remote parameter looks fine for me now. Yes there is a possibility of deleting not merged remote branches but at least it is simple. One just need to push to remote before deleting branches. So now I'm think about reverting 0f51d0d and 74a3cf5. Really don't know which one is better.

dahlbyk · 2015-05-31T19:29:25Z

I'm not really a fan of doing any sort of remote manipulation based on local names because upstream branches don't necessarily have to track a local branch of the same name. I suppose you could remove remote branches that have been merged into the remote branch tracked by the current local branch, if applicable, but that feels like a stretch?

Since we don't specify a reference to the commit we're checking is merged, I can really only think of two reasonable behaviors for Remove-MergedGitBranches -Remote origin:

Remove remote branches that have been merged into local HEAD (i.e. $(git branch -r --merged) -like ' origin/*').
Remove remote branches that have been merged into origin/HEAD (the default branch, typically master but configurable).

Between the two I lean toward the latter, with an error if <remote>/HEAD does not exist (it's typically created by git clone but not git remote add).

More generally, earlier I asked:

General question: would it be useful for any/all of these to accept a parameter (defaulting to HEAD) to identify the ref to check mergedness against...?

Especially for the -Remote scenario the answer seems to be "Yes".

This parameter identify the ref to check mergedness against.

tkirill · 2015-06-05T07:45:09Z

@dahlbyk I tried to implement -Target parameter in 7658e2f and it looks nice. Here is current interface:

-Local -- remove local branches.
-Remote origin, other -- set remotes
-Target branch -- set target branch. HEAD is default.
No parameters equals to -Local

-Target parameter is positional so Remove-MergedGitBranches branch1 removes local branches merged to branch1.

... with an error if /HEAD does not exist ...

I write it as warning since it is not fatal. What do you think? This is how it looks:

Also, I added a test with multiple remotes and custom target branch. These cases are currently tested:

Do nothing if no branches: local & remote
Do nothing if branch not merged: local & remote
Remove single merged branch: local & remote
Do nothing if branch is merged but it is a current branch: local
Pass HEAD explicitly: local
Custom branch as a target: local & remote
Multiple merged branches: local & remote
If branch merged locally and not merged remotely then remove only local branch: test
If branch merged remotely and not merged locally then remove only remote branch: test
If branch merged remotely and locally then remove both local and remote branch: test
Multiple remotes:

dahlbyk · 2015-06-05T08:46:09Z

No time to review code now, but API seems just right and test coverage is impressive! Will try to revisit over the weekend - bug me next week if I don't.

Thanks for iterating on this!

tkirill · 2015-06-13T11:10:13Z

bug me next week if I don't.

@dahlbyk 👋

tkirill · 2015-11-18T07:18:40Z

Just discovered one more corner case.

If we have branch which points at the same commit as develop then this branch considered merged and the script removes it. However, it can be just new branch that has no commits yet. Removing such branch surprised my colleague who created this branch 😆

I don't know if we could fix this since the same situation can occur with fast-forward merge.

Also, after few month of using current Remove-MergedGitBranches I have feeling that its interface isn't convenient. Maybe it isn't true but it would be good to review it again.

rkeithhill · 2019-02-24T22:42:13Z

Closing this in favor of PR #663.

tkirill added 3 commits October 2, 2014 15:41

Add parameter 'Ignore' to override default list of ignored branches.

d9c7f09

Default list is master and develop.

Add support for -WhatIf and -Confirm.

f666901

dahlbyk reviewed Nov 16, 2014
View reviewed changes

tkirill added 3 commits November 19, 2014 19:36

Make filter from TrimRemote-GitBranch.

e093459

Extract default list of merged branches to keep to GitPromtSettings.M…

58cab91

…ergedBranchesToKeep.

Remove debug messages.

70efe3e

bcdady mentioned this pull request Apr 26, 2015

Remove remote merged branches command #79

Closed

dahlbyk reviewed May 14, 2015
View reviewed changes

tkirill added 3 commits May 22, 2015 17:32

Remove unused parameter "Ignore" from Remove-MergedGitBranches.

47e6c16

I haven't used it in the last 6 month. This functionality is likely redundant.

Add parameter "RemoteName" to Remove-MergedGitBranches.

98956d1

Default value is "origin". Also, it is not required to pass PSBoundParameters. Is works automatically.

tkirill added 2 commits May 22, 2015 18:55

Merge branch 'master' into DeleteMergedBranches

c441b08

Conflicts: GitPrompt.ps1

Reformat GitRemoveMergedBranches.ps1.

7cf9803

tkirill reviewed May 22, 2015
View reviewed changes

tkirill force-pushed the DeleteMergedBranches branch 2 times, most recently from 9287c11 to 23e23a2 Compare May 23, 2015 16:03

Add support for multiple remotes in Remove-MergedGitBranches.

1b881b5

Default behavior is to delete remote branches in origin only.

tkirill force-pushed the DeleteMergedBranches branch from 23e23a2 to 1b881b5 Compare May 23, 2015 16:05

Remove Write-Host's that duplicate PSCmdlet.ShouldProcess.

0d42639

tkirill added 2 commits May 27, 2015 18:44

New way of working with remote branches in Remove-MergedGitBranches.

74a3cf5

Now cmdlet requests target branch in `-RemoteBranch` parameter. It checks merged branches against target in the same remote as target branch.

tkirill force-pushed the DeleteMergedBranches branch from 2e6b470 to 74a3cf5 Compare May 27, 2015 17:13

Add -Target parameter for Remove-MergedGitBranches.

7658e2f

This parameter identify the ref to check mergedness against.

tkirill force-pushed the DeleteMergedBranches branch from e016be9 to 7658e2f Compare June 5, 2015 07:21

dahlbyk added this to the Ideas milestone Feb 6, 2017

dahlbyk force-pushed the master branch from 0cddf55 to 53c9145 Compare September 29, 2017 06:33

dahlbyk mentioned this pull request Jan 1, 2018

Feature Idea: Helper to delete branches that exist only locally but not on the remote #507

Closed

rkeithhill closed this Feb 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove merged branches #159

Remove merged branches #159

tkirill commented Oct 2, 2014

dahlbyk commented Nov 16, 2014

dahlbyk Nov 16, 2014

tkirill Nov 19, 2014

dahlbyk commented Nov 16, 2014

tkirill commented Nov 19, 2014

tkirill commented May 14, 2015

dahlbyk May 14, 2015

tkirill May 22, 2015

dahlbyk commented May 14, 2015

tkirill commented May 22, 2015

tkirill May 22, 2015

dahlbyk May 23, 2015

dahlbyk commented May 23, 2015

tkirill commented May 23, 2015

dahlbyk commented May 23, 2015

tkirill commented May 24, 2015

dahlbyk commented May 24, 2015

tkirill commented May 27, 2015

tkirill commented May 31, 2015

dahlbyk commented May 31, 2015

tkirill commented Jun 5, 2015

dahlbyk commented Jun 5, 2015

tkirill commented Jun 13, 2015

tkirill commented Nov 18, 2015

rkeithhill commented Feb 24, 2019

Remove merged branches #159

Remove merged branches #159

Conversation

tkirill commented Oct 2, 2014

dahlbyk commented Nov 16, 2014

dahlbyk Nov 16, 2014

Choose a reason for hiding this comment

tkirill Nov 19, 2014

Choose a reason for hiding this comment

dahlbyk commented Nov 16, 2014

tkirill commented Nov 19, 2014

tkirill commented May 14, 2015

dahlbyk May 14, 2015

Choose a reason for hiding this comment

tkirill May 22, 2015

Choose a reason for hiding this comment

dahlbyk commented May 14, 2015

tkirill commented May 22, 2015

tkirill May 22, 2015

Choose a reason for hiding this comment

dahlbyk May 23, 2015

Choose a reason for hiding this comment

dahlbyk commented May 23, 2015

tkirill commented May 23, 2015

dahlbyk commented May 23, 2015

tkirill commented May 24, 2015

dahlbyk commented May 24, 2015

tkirill commented May 27, 2015

tkirill commented May 31, 2015

dahlbyk commented May 31, 2015

tkirill commented Jun 5, 2015

dahlbyk commented Jun 5, 2015

tkirill commented Jun 13, 2015

tkirill commented Nov 18, 2015

rkeithhill commented Feb 24, 2019