fix #451 #459
fix #451 #459
Conversation
mob.go
Outdated
| @@ -551,6 +566,11 @@ func start(configuration config.Configuration) error { | |||
|
|
|||
| createRemoteBranch(configuration, currentBaseBranch) | |||
|
|
|||
| if !currentBaseBranch.hasLocalBranch(localBranches) { | |||
| silentgit("checkout", "-b", currentBaseBranch.Name) | |||
| silentgit("checkout", currentBranch.Name) | |||
There was a problem hiding this comment.
why do we need this line? It feels wrong
There was a problem hiding this comment.
Hi @simonharrer, first I thought we need it, because the git command which is used to check if there are unpushed commits. But because of your comment I thought more about it and if there is no local base branch there can not be unpushed commits. but after implementing this change the test failed again. In the function sayLastCommitsList we look at the local basebranch. I now implemented it the way that we try this behavior if it fails we try to check against remote. Let me know if you think it is better to directly compare to the remote base branch instead of trying to compare to the local branch first. I think it could make sense but was not completely sure about it.
| @@ -1001,12 +1016,16 @@ func squashOrCommit(configuration config.Configuration) string { | |||
| } | |||
| } | |||
|
|
|||
| func sayLastCommitsList(currentBaseBranch string, currentWipBranch string) { | |||
| commitsBaseWipBranch := currentBaseBranch + ".." + currentWipBranch | |||
There was a problem hiding this comment.
@simonharrer this is the place where we could also just use the currentRemoteBaseBranch, this would be a much easier solution.
mob.go
Outdated
| @@ -122,6 +122,19 @@ func (branch Branch) hasRemoteBranch(configuration config.Configuration) bool { | |||
| return false | |||
| } | |||
|
|
|||
| func (branch Branch) hasLocalBranch(localBranches []string) bool { | |||
There was a problem hiding this comment.
Imo, hasLocalBranch should not need an argument localBranches. It can fetch them itself. And if local branches are fetched multiple times, I still think its worth it. When I need to find out how to check if a localBranch exists in our code, and I find this function, I don't want it to make me search for yet another function.
There was a problem hiding this comment.
The same is probably applicable to determineBranches
No description provided.