bug: don't make bug actions drive identity actions

Michael Muré created 6 years ago

Turns out it was a mistake, who is suprised ?

It leaks the abstraction and it's generally much cleaner to let the cache layer make both calls

Change summary

bug/bug_actions.go      | 46 ++----------------------------------------
bug/bug_actions_test.go | 25 +++++++++++++++++++++++
2 files changed, 28 insertions(+), 43 deletions(-)

Detailed changes

bug/bug_actions.go

@@ -4,68 +4,28 @@ import (
 	"fmt"
 	"strings"
 
-	"github.com/MichaelMure/git-bug/identity"
 	"github.com/MichaelMure/git-bug/repository"
 	"github.com/pkg/errors"
 )
 
-// Note:
-//
-// For the actions (fetch/push/pull/merge/commit), this package act as a master for
-// the identity package and will also drive the needed identity actions. That is,
-// if bug.Push() is called, identity.Push will also be called to make sure that
-// the dependant identities are also present and up to date on the remote.
-//
-// I'm not entirely sure this is the correct way to do it, as it might introduce
-// too much complexity and hard coupling, but it does make this package easier
-// to use.
-
 // Fetch retrieve updates from a remote
 // This does not change the local bugs state
 func Fetch(repo repository.Repo, remote string) (string, error) {
-	stdout, err := identity.Fetch(repo, remote)
-	if err != nil {
-		return stdout, err
-	}
-
 	remoteRefSpec := fmt.Sprintf(bugsRemoteRefPattern, remote)
 	fetchRefSpec := fmt.Sprintf("%s*:%s*", bugsRefPattern, remoteRefSpec)
 
-	stdout2, err := repo.FetchRefs(remote, fetchRefSpec)
-
-	return stdout + "\n" + stdout2, err
+	return repo.FetchRefs(remote, fetchRefSpec)
 }
 
 // Push update a remote with the local changes
 func Push(repo repository.Repo, remote string) (string, error) {
-	stdout, err := identity.Push(repo, remote)
-	if err != nil {
-		return stdout, err
-	}
-
-	stdout2, err := repo.PushRefs(remote, bugsRefPattern+"*")
-
-	return stdout + "\n" + stdout2, err
+	return repo.PushRefs(remote, bugsRefPattern+"*")
 }
 
 // Pull will do a Fetch + MergeAll
 // This function will return an error if a merge fail
 func Pull(repo repository.ClockedRepo, remote string) error {
-	_, err := identity.Fetch(repo, remote)
-	if err != nil {
-		return err
-	}
-
-	for merge := range identity.MergeAll(repo, remote) {
-		if merge.Err != nil {
-			return merge.Err
-		}
-		if merge.Status == identity.MergeStatusInvalid {
-			return errors.Errorf("merge failure: %s", merge.Reason)
-		}
-	}
-
-	_, err = Fetch(repo, remote)
+	_, err := Fetch(repo, remote)
 	if err != nil {
 		return err
 	}

bug/bug_actions_test.go

@@ -20,6 +20,12 @@ func TestPushPull(t *testing.T) {
 	err = bug1.Commit(repoA)
 	require.NoError(t, err)
 
+	// distribute the identity
+	_, err = identity.Push(repoA, "origin")
+	require.NoError(t, err)
+	err = identity.Pull(repoB, "origin")
+	require.NoError(t, err)
+
 	// A --> remote --> B
 	_, err = Push(repoA, "origin")
 	require.NoError(t, err)
@@ -87,7 +93,14 @@ func _RebaseTheirs(t testing.TB) {
 	err = bug1.Commit(repoA)
 	require.NoError(t, err)
 
+	// distribute the identity
+	_, err = identity.Push(repoA, "origin")
+	require.NoError(t, err)
+	err = identity.Pull(repoB, "origin")
+	require.NoError(t, err)
+
 	// A --> remote
+
 	_, err = Push(repoA, "origin")
 	require.NoError(t, err)
 
@@ -153,6 +166,12 @@ func _RebaseOurs(t testing.TB) {
 	err = bug1.Commit(repoA)
 	require.NoError(t, err)
 
+	// distribute the identity
+	_, err = identity.Push(repoA, "origin")
+	require.NoError(t, err)
+	err = identity.Pull(repoB, "origin")
+	require.NoError(t, err)
+
 	// A --> remote
 	_, err = Push(repoA, "origin")
 	require.NoError(t, err)
@@ -236,6 +255,12 @@ func _RebaseConflict(t testing.TB) {
 	err = bug1.Commit(repoA)
 	require.NoError(t, err)
 
+	// distribute the identity
+	_, err = identity.Push(repoA, "origin")
+	require.NoError(t, err)
+	err = identity.Pull(repoB, "origin")
+	require.NoError(t, err)
+
 	// A --> remote
 	_, err = Push(repoA, "origin")
 	require.NoError(t, err)