From dccd499d625618d22c06a004f375a50770063f2e Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 11 Nov 2022 12:42:27 +0000 Subject: [PATCH] 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 --- git/gogit/client.go | 34 ++++++++++++++++++++-------------- git/gogit/client_test.go | 2 +- git/gogit/clone_test.go | 4 ++-- git/libgit2/client.go | 26 ++++++++++++++++---------- git/libgit2/clone_test.go | 4 ++-- 5 files changed, 41 insertions(+), 29 deletions(-) diff --git a/git/gogit/client.go b/git/gogit/client.go index d976e117..bfae32b9 100644 --- a/git/gogit/client.go +++ b/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 { diff --git a/git/gogit/client_test.go b/git/gogit/client_test.go index a36ecaa5..3bdd8e24 100644 --- a/git/gogit/client_test.go +++ b/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 diff --git a/git/gogit/clone_test.go b/git/gogit/clone_test.go index 27a1253c..65cf4c4c 100644 --- a/git/gogit/clone_test.go +++ b/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{ diff --git a/git/libgit2/client.go b/git/libgit2/client.go index 46fb0dfa..776a5e87 100644 --- a/git/libgit2/client.go +++ b/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 { diff --git a/git/libgit2/clone_test.go b/git/libgit2/clone_test.go index dd8c707f..b4965bb7 100644 --- a/git/libgit2/clone_test.go +++ b/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{