Repository: remotemobprogramming/mob
Pull request #222 (2021-12-19 17:01:28)

No squash
When the user has uncommitted changes and runs `mob done --no-squash`, the changes get commited in a wip commit, but this wip commit gets reset softly after the merge, so the user gets the chance to commit the changes manually with a proper message. 

Tackles this Issue: https://github.com/remotemobprogramming/mob/issues/219 


Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index c8ce88b..5293cf7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,6 +1,7 @@
 # 2.2.0
 - When mob encounters unpushed commits in the base branch, the tool now provides help for the user to fix this immediately.
 - Improves console output when using `mob break 5` in combination with https://timer.mob.sh
+- Gives the user the chance to name their final commit after `mob done --no-squash`
 
 # 2.1.0
 - When having set `MOB_TIMER_ROOM` the local timer keeps on working. To disable the local timer altogether, please disable it via `export MOB_TIMER_LOCAL=false`.
diff --git a/mob.go b/mob.go
index 7ede5c1..7d3908c 100644
--- a/mob.go
+++ b/mob.go
@@ -985,7 +985,8 @@ func done(configuration Configuration) {
 	baseBranch, wipBranch := determineBranches(gitCurrentBranch(), gitBranches(), configuration)
 
 	if wipBranch.hasRemoteBranch(configuration) {
-		if hasUncommittedChanges() {
+		uncommittedChanges := hasUncommittedChanges()
+		if uncommittedChanges {
 			makeWipCommit(configuration)
 		}
 		git("push", "--no-verify", configuration.RemoteName, wipBranch.Name)
@@ -998,6 +999,11 @@ func done(configuration Configuration) {
 		}
 
 		git("branch", "-D", wipBranch.Name)
+
+		if uncommittedChanges && !configuration.MobDoneSquash { // give the user the chance to name their final commit
+			git("reset", "--soft", "HEAD^")
+		}
+
 		git("push", "--no-verify", configuration.RemoteName, "--delete", wipBranch.Name)
 
 		cachedChanges := getCachedChanges()
@@ -1009,12 +1015,11 @@ func done(configuration Configuration) {
 		if err != nil {
 			sayError(err.Error())
 		}
-		if configuration.MobDoneSquash {
-			if isNothingToCommit() {
-				sayInfo("nothing was done, so nothing to commit")
-			} else {
-				sayTodo("To finish, use", "git commit")
-			}
+
+		if hasUncommittedChanges() {
+			sayTodo("To finish, use", "git commit")
+		} else if configuration.MobDoneSquash {
+			sayInfo("nothing was done, so nothing to commit")
 		}
 
 	} else {


Test patch:
diff --git a/mob_test.go b/mob_test.go
index e87c920..7436a75 100644
--- a/mob_test.go
+++ b/mob_test.go
@@ -667,8 +667,7 @@ func TestStartDoneWithMobDoneSquashFalse(t *testing.T) {
 
 func TestStartDonePublishingOneManualCommit(t *testing.T) {
 	_, configuration := setup(t)
-	// REFACTOR Replace string with enum value
-	configuration.MobDoneSquash = false // default is probably true
+	configuration.MobDoneSquash = false // default is true
 
 	start(configuration)
 	assertOnBranch(t, "mob-session")
@@ -680,6 +679,7 @@ func TestStartDonePublishingOneManualCommit(t *testing.T) {
 	done(configuration) // without squash (configuration)
 
 	assertOnBranch(t, "master")
+	assertCleanGitStatus(t)
 	assertCommitsOnBranch(t, 2, "master")
 	assertCommitLogContainsMessage(t, "master", "[manual-commit-1] publish this commit to master")
 	assertCommitsOnBranch(t, 1, "origin/master")
@@ -688,7 +688,6 @@ func TestStartDonePublishingOneManualCommit(t *testing.T) {
 
 func TestStartDoneSquashTheOneManualCommit(t *testing.T) {
 	_, configuration := setup(t)
-	// REFACTOR Replace string with enum value
 	configuration.MobDoneSquash = true
 
 	start(configuration)
@@ -700,10 +699,45 @@ func TestStartDoneSquashTheOneManualCommit(t *testing.T) {
 
 	done(configuration)
 
+	assertOnBranch(t, "master")
 	assertGitStatus(t, GitStatus{
 		"example.txt": "A",
 	})
+	assertCommitsOnBranch(t, 1, "master")
+	assertCommitsOnBranch(t, 1, "origin/master")
+	assertNoMobSessionBranches(t, configuration, "mob-session")
+}
+
+func TestStartDoneWithUncommittedChanges(t *testing.T) {
+	_, configuration := setup(t)
+
+	start(configuration) // should be 1 commit on mob-session so far
+	createFile(t, "example.txt", "content")
+
+	done(configuration)
+
+	assertOnBranch(t, "master")
+	assertGitStatus(t, GitStatus{
+		"example.txt": "A",
+	})
+	assertCommitsOnBranch(t, 1, "master")
+	assertCommitsOnBranch(t, 1, "origin/master")
+	assertNoMobSessionBranches(t, configuration, "mob-session")
+}
+
+func TestStartDoneNoSquashWithUncommittedChanges(t *testing.T) {
+	_, configuration := setup(t)
+	configuration.MobDoneSquash = false // default is true
+
+	start(configuration) // should be 1 commit on mob-session so far
+	createFile(t, "example.txt", "content")
+
+	done(configuration) // without squash (configuration)
+
 	assertOnBranch(t, "master")
+	assertGitStatus(t, GitStatus{
+		"example.txt": "A",
+	})
 	assertCommitsOnBranch(t, 1, "master")
 	assertCommitsOnBranch(t, 1, "origin/master")
 	assertNoMobSessionBranches(t, configuration, "mob-session")
@@ -980,6 +1014,7 @@ func TestEmptyGitStatus(t *testing.T) {
 	status := gitStatus()
 
 	equals(t, 0, len(status))
+	assertCleanGitStatus(t)
 }
 
 func TestGitStatusWithOneFile(t *testing.T) {
@@ -1167,6 +1202,15 @@ func assertGitStatus(t *testing.T, expected map[string]string) {
 	equals(t, expected, gitStatus())
 }
 
+func assertCleanGitStatus(t *testing.T) {
+	status := gitStatus()
+	if len(status) != 0 {
+		_, file, line, _ := runtime.Caller(1)
+		fmt.Printf("\033[31m%s:%d:\n\n\texpected a clean git status, but contained %s\"\n", filepath.Base(file), line, status)
+		t.FailNow()
+	}
+}
+
 func equals(t *testing.T, exp, act interface{}) {
 	if !reflect.DeepEqual(exp, act) {
 		t.Log(string(debug.Stack()))