git: Consolidate the use of ClientOption

The use of options were a mix of value and funcs, which in some cases
was confusing:

NewClient(os.TempDir(), nil,
	WithDiskStorage,
	WithForcePush(),
	WithInsecureCredentialsOverHTTP)

With the changes, all options are exposed as funcs instead:

NewClient(os.TempDir(), nil,
	WithDiskStorage(),
	WithForcePush(),
	WithInsecureCredentialsOverHTTP())

The above changes aligns with the standards used in source controller
internal/reconcile/summarize.

Signed-off-by: Paulo Gomes <paulo.gomes@weave.works>

git/gogit/client.go

@@ -73,7 +73,7 @@ func NewClient(path string, authOpts *git.AuthOptions, clientOpts ...ClientOptio
 	}
 
 	if len(clientOpts) == 0 {
-		clientOpts = append(clientOpts, WithDiskStorage)
+		clientOpts = append(clientOpts, WithDiskStorage())
 	}
 
 	for _, clientOpt := range clientOpts {
@@ -106,19 +106,23 @@ func WithWorkTreeFS(wt billy.Filesystem) ClientOption {
 	}
 }
 
-func WithDiskStorage(g *Client) error {
-	wt := fs.New(g.path)
-	dot := fs.New(filepath.Join(g.path, extgogit.GitDirName))
+func WithDiskStorage() ClientOption {
+	return func(c *Client) error {
+		wt := fs.New(c.path)
+		dot := fs.New(filepath.Join(c.path, extgogit.GitDirName))
 
-	g.storer = filesystem.NewStorage(dot, cache.NewObjectLRUDefault())
-	g.worktreeFS = wt
-	return nil
+		c.storer = filesystem.NewStorage(dot, cache.NewObjectLRUDefault())
+		c.worktreeFS = wt
+		return nil
+	}
 }
 
-func WithMemoryStorage(g *Client) error {
-	g.storer = memory.NewStorage()
-	g.worktreeFS = memfs.New()
-	return nil
+func WithMemoryStorage() ClientOption {
+	return func(c *Client) error {
+		c.storer = memory.NewStorage()
+		c.worktreeFS = memfs.New()
+		return nil
+	}
 }
 
 // WithForcePush enables the use of force push for all push operations
@@ -133,9 +137,11 @@ func WithForcePush() ClientOption {
 
 // WithInsecureCredentialsOverHTTP enables credentials being used over
 // HTTP. This is not recommended for production environments.
-func WithInsecureCredentialsOverHTTP(g *Client) error {
-	g.credentialsOverHTTP = true
-	return nil
+func WithInsecureCredentialsOverHTTP() ClientOption {
+	return func(c *Client) error {
+		c.credentialsOverHTTP = true
+		return nil
+	}
 }
 
 func (g *Client) Init(ctx context.Context, url, branch string) error {

git/gogit/client_test.go

@@ -275,7 +275,7 @@ func TestForcePush(t *testing.T) {
 	cc2, err := commitFile(repo2, "test", "first push from second clone", time.Now())
 	g.Expect(err).ToNot(HaveOccurred())
 
-	ggc2, err := NewClient(tmp2, nil, WithDiskStorage, WithForcePush())
+	ggc2, err := NewClient(tmp2, nil, WithDiskStorage(), WithForcePush())
 	g.Expect(err).ToNot(HaveOccurred())
 	ggc2.repository = repo2
 

git/gogit/clone_test.go

@@ -1038,9 +1038,9 @@ func TestClone_CredentialsOverHttp(t *testing.T) {
 			previousRequestCount = totalRequests
 
 			tmpDir := t.TempDir()
-			opts := []ClientOption{WithDiskStorage}
+			opts := []ClientOption{WithDiskStorage()}
 			if tt.allowCredentialsOverHttp {
-				opts = append(opts, WithInsecureCredentialsOverHTTP)
+				opts = append(opts, WithInsecureCredentialsOverHTTP())
 			}
 
 			ggc, err := NewClient(tmpDir, &git.AuthOptions{

git/libgit2/client.go

@@ -79,7 +79,7 @@ func NewClient(path string, authOpts *git.AuthOptions, clientOpts ...ClientOptio
 	}
 
 	if len(clientOpts) == 0 {
-		clientOpts = append(clientOpts, WithDiskStorage)
+		clientOpts = append(clientOpts, WithDiskStorage())
 	}
 
 	for _, clientOpt := range clientOpts {
@@ -95,21 +95,27 @@ func NewClient(path string, authOpts *git.AuthOptions, clientOpts ...ClientOptio
 	return l, nil
 }
 
-func WithDiskStorage(l *Client) error {
-	l.repoFS = osfs.New(l.path)
-	return nil
+func WithDiskStorage() ClientOption {
+	return func(c *Client) error {
+		c.repoFS = osfs.New(c.path)
+		return nil
+	}
 }
 
-func WithMemoryStorage(l *Client) error {
-	l.repoFS = memfs.New()
-	return nil
+func WithMemoryStorage() ClientOption {
+	return func(c *Client) error {
+		c.repoFS = memfs.New()
+		return nil
+	}
 }
 
 // WithInsecureCredentialsOverHTTP enables credentials being used over
 // HTTP. This is not recommended for production environments.
-func WithInsecureCredentialsOverHTTP(l *Client) error {
-	l.credentialsOverHTTP = true
-	return nil
+func WithInsecureCredentialsOverHTTP() ClientOption {
+	return func(c *Client) error {
+		c.credentialsOverHTTP = true
+		return nil
+	}
 }
 
 func (l *Client) Init(ctx context.Context, url, branch string) error {

git/libgit2/clone_test.go

@@ -647,9 +647,9 @@ func TestClone_CredentialsOverHttp(t *testing.T) {
 			previousRequestCount = totalRequests
 
 			tmpDir := t.TempDir()
-			opts := []ClientOption{WithDiskStorage}
+			opts := []ClientOption{WithDiskStorage()}
 			if tt.allowCredentialsOverHttp {
-				opts = append(opts, WithInsecureCredentialsOverHTTP)
+				opts = append(opts, WithInsecureCredentialsOverHTTP())
 			}
 
 			lgc, err := NewClient(tmpDir, &git.AuthOptions{