-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
fix #451 #459
fix #451 #459
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ import ( | |
) | ||
|
||
const ( | ||
versionNumber = "5.3.2" | ||
versionNumber = "5.3.3" | ||
minimumGitVersion = "2.13.0" | ||
) | ||
|
||
|
@@ -122,6 +122,19 @@ func (branch Branch) hasRemoteBranch(configuration config.Configuration) bool { | |
return false | ||
} | ||
|
||
func (branch Branch) hasLocalBranch(localBranches []string) bool { | ||
say.Debug("Local Branches: " + strings.Join(localBranches, "\n")) | ||
say.Debug("Local Branch: " + branch.Name) | ||
|
||
for i := 0; i < len(localBranches); i++ { | ||
if localBranches[i] == branch.Name { | ||
return true | ||
} | ||
} | ||
|
||
return false | ||
} | ||
|
||
func (branch Branch) IsWipBranch(configuration config.Configuration) bool { | ||
if branch.Name == "mob-session" { | ||
return true | ||
|
@@ -536,7 +549,9 @@ func start(configuration config.Configuration) error { | |
} | ||
|
||
git("fetch", configuration.RemoteName, "--prune") | ||
currentBaseBranch, currentWipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration) | ||
currentBranch := gitCurrentBranch() | ||
localBranches := gitBranches() | ||
currentBaseBranch, currentWipBranch := determineBranches(currentBranch, localBranches, configuration) | ||
|
||
if !currentWipBranch.hasRemoteBranch(configuration) && configuration.StartJoin { | ||
say.Error("Remote wip branch " + currentWipBranch.remote(configuration).String() + " is missing") | ||
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need this line? It feels wrong There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
} | ||
|
||
if currentBaseBranch.hasUnpushedCommits(configuration) { | ||
say.Error("cannot start; unpushed changes on base branch must be pushed upstream") | ||
say.Fix("to fix this, push those commits and try again", "git push "+configuration.RemoteName+" "+currentBaseBranch.String()) | ||
|
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same is probably applicable to determineBranches