-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Delete range selection of branches #4073
base: master
Are you sure you want to change the base?
Conversation
Besides being a useful cleanup on its own, it will make it easier to support a multiselection of branches.
Since we want to select multiselections, this will make it easier to pass a slice of remote branches. It does require that for the case of the local branches panel we need to synthesize a RemoteBranch object from the selected local branch, but that's not hard.
We'll need it a few more times in the next test we add.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesCodacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more Footnotes
|
We allow deleting remote branches (or local and remote branches) only if *all* selected branches have one. We show the a warning about force-deleting as soon as at least one of the selected branches is not fully merged. The added test only tests a few of the most interesting cases; I didn't try to cover the whole space of possible combinations, that would have been too much.
e144aad
to
99e4112
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yass! This is exactly the behavior I was expecting. Thanks a lot! Hope it gets released soon.
Fixes #4041
if len(branches) > 1 { | ||
if lo.SomeBy(branches, func(branch *models.Branch) bool { return self.checkedOutByOtherWorktree(branch) }) { | ||
return errors.New(self.c.Tr.SomeBranchesCheckedOutByWorktreeError) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we completely disallowing branch deletion if it's checked out by another worktree? I noticed the previous implementation prompted the user first rather than outright rejecting it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If one of the selected branches is checked out in another worktree, yes. I did this because I found it too complex to wrap my head around what should happen in that case (for a range selection).
If you select those branches one by one, you still get the prompt for what to do with the worktree as before.
This allows range-selecting multiple branches and deleting them all at once. We allow deleting remote branches (or local and remote branches) as long as all selected branches have one.
We show the warning about force-deleting as soon as at least one of the selected branches is not fully merged.
go generate ./...
)