Merge pull request #20 from fluxcd/cleanup

Various cleanup fixes

.github/workflows/golangci.yaml

@@ -1,6 +1,10 @@
 # https://github.com/marketplace/actions/run-golangci-lint
 name: golangci-lint
-on: [push, pull_request]
+on:
+  pull_request:
+  push:
+    branches:
+      - master
 jobs:
   golangci:
     name: lint
@@ -11,4 +15,4 @@ jobs:
         uses: golangci/golangci-lint-action@v1
         with:
           # Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
-          version: v1.28
+          version: v1.30

.golangci.yml

@@ -0,0 +1,59 @@
+run:
+  # timeout for analysis, e.g. 30s, 5m, default is 1m
+  timeout: 1m
+  tests: false
+
+linters:
+  enable:
+  # Default
+  - govet
+  - errcheck
+  - staticcheck
+  - unused
+  - gosimple
+  - structcheck
+  - varcheck
+  - ineffassign
+  - deadcode
+  - typecheck
+  # Extra, see list of https://golangci-lint.run/usage/linters/
+  - bodyclose
+  - golint
+  # - rowserrcheck is not applicable here
+  - stylecheck
+  - gosec
+  # - interfacer is deprecated
+  - unconvert
+  - dupl
+  - goconst
+  - gocyclo
+  - gocognit
+  - asciicheck
+  - gofmt
+  - goimports
+  - maligned
+  # - depguard is not applicable here
+  - misspell
+  # - lll allow long lines for now, maybe revisit this later
+  - unparam
+  - dogsled
+  - nakedret
+  - prealloc
+  - scopelint
+  - gocritic
+  - gochecknoinits
+  - gochecknoglobals
+  # - godox don't error although there are TODOs in the code
+  - funlen
+  - whitespace
+  # - wsl allow "non-idiomatic" whitespace formattings for now
+  - goprintffuncname
+  - gomnd
+  - goerr113
+  # - gomodguard is not applicable here
+  - godot
+  # - testpackage is not applicable here
+  # - nestif is covered by gocognit
+  # - exportloopref is covered by scopelint
+  - exhaustive
+  - nolintlint

README.md

@@ -2,5 +2,11 @@
 
 [![godev](https://img.shields.io/static/v1?label=godev&message=reference&color=00add8)](https://pkg.go.dev/github.com/fluxcd/go-git-providers)
 [![build](https://github.com/fluxcd/go-git-providers/workflows/build/badge.svg)](https://github.com/fluxcd/go-git-providers/actions)
+[![Go Report Card](https://goreportcard.com/badge/github.com/fluxcd/go-git-providers)](https://goreportcard.com/report/github.com/fluxcd/go-git-providers)
+[![codecov.io](https://codecov.io/github/fluxcd/go-git-providers/coverage.svg?branch=master)](https://codecov.io/github/fluxcd/go-git-providers?branch=master)
+[![LICENSE](https://img.shields.io/github/license/fluxcd/go-git-providers)](https://github.com/fluxcd/go-git-providers/blob/master/LICENSE)
+[![Contributors](https://img.shields.io/github/contributors/fluxcd/go-git-providers)](https://github.com/fluxcd/go-git-providers/graphs/contributors)
+[![Release](https://img.shields.io/github/v/release/fluxcd/go-git-providers?include_prereleases)](https://github.com/fluxcd/go-git-providers/releases/latest)
+[![PRs Welcome](https://img.shields.io/badge/PRs-welcome-brightgreen.svg?style=flat-square)](https://github.com/fluxcd/go-git-providers/blob/master/CONTRIBUTING.md)
 
 The go-git-providers is a general-purpose Go client for interacting with Git providers APIs (e.g. GitHub, GitLab, Bitbucket).

bitbucket/doc.go

@@ -17,5 +17,6 @@ limitations under the License.
 package bitbucket
 
 import (
+	// TODO: Dummy import until we have the implementation ready.
 	_ "github.com/ktrysmt/go-bitbucket"
 )

github/auth.go

@@ -29,7 +29,7 @@ import (
 )
 
 const (
-	// defaultDomain specifies the default domain used as the backend
+	// defaultDomain specifies the default domain used as the backend.
 	defaultDomain = "github.com"
 	// patUsername is the "username" for the basic auth authentication flow
 	// when using a personal access token as the "password". This string could
@@ -40,21 +40,21 @@ const (
 
 var (
 	// ErrInvalidClientOptions is the error returned when calling NewClient() with
-	// invalid options (e.g. specifying mutually exclusive options)
+	// invalid options (e.g. specifying mutually exclusive options).
 	ErrInvalidClientOptions = errors.New("invalid options given to NewClient()")
 	// ErrDestructiveCallDisallowed happens when the client isn't set up with WithDestructiveAPICalls()
 	// but a destructive action is called.
 	ErrDestructiveCallDisallowed = errors.New("a destructive call was blocked because it wasn't allowed by the client")
 )
 
 // clientOptions is the struct that tracks data about what options have been set
-// It is private so that the user must use the With... functions
+// It is private so that the user must use the With... functions.
 type clientOptions struct {
 	// Domain specifies the backing domain, which can be arbitrary if the user uses
 	// GitHub Enterprise. If unset, defaultDomain will be used.
 	Domain *string
 
-	// ClientFactory is a way to aquire a *http.Client, possibly with auth credentials
+	// ClientFactory is a way to acquire a *http.Client, possibly with auth credentials
 	ClientFactory ClientFactory
 
 	// EnableDestructiveAPICalls is a flag to tell whether destructive API calls like
@@ -79,6 +79,7 @@ func WithOAuth2Token(oauth2Token string) ClientOption {
 		if opts.ClientFactory != nil {
 			return fmt.Errorf("authentication http.Client already configured: %w", ErrInvalidClientOptions)
 		}
+
 		opts.ClientFactory = &oauth2Auth{oauth2Token}
 		return nil
 	}
@@ -102,7 +103,7 @@ func WithPersonalAccessToken(patToken string) ClientOption {
 	}
 }
 
-// WithClientFactory initializes a Client with a given ClientFactory, used for aquiring the *http.Client later.
+// WithClientFactory initializes a Client with a given ClientFactory, used for acquiring the *http.Client later.
 // clientFactory must not be nil.
 // WithClientFactory is mutually exclusive with WithOAuth2Token and WithPersonalAccessToken.
 func WithClientFactory(clientFactory ClientFactory) ClientOption {
@@ -150,31 +151,31 @@ func WithDestructiveAPICalls(destructiveActions bool) ClientOption {
 	}
 }
 
-// ClientFactory is a way to aquire a *http.Client, possibly with auth credentials
+// ClientFactory is a way to acquire a *http.Client, possibly with auth credentials.
 type ClientFactory interface {
-	// Client returns a *http.Client, possibly with auth credentials
+	// Client returns a *http.Client, possibly with auth credentials.
 	Client(ctx context.Context) *http.Client
 }
 
-// oauth2Auth is an implementation of ClientFactory
+// oauth2Auth is an implementation of ClientFactory.
 type oauth2Auth struct {
 	token string
 }
 
-// Client returns a *http.Client, possibly with auth credentials
+// Client returns a *http.Client, possibly with auth credentials.
 func (a *oauth2Auth) Client(ctx context.Context) *http.Client {
 	ts := oauth2.StaticTokenSource(
 		&oauth2.Token{AccessToken: a.token},
 	)
 	return oauth2.NewClient(ctx, ts)
 }
 
-// patAuth is an implementation of ClientFactory
+// patAuth is an implementation of ClientFactory.
 type patAuth struct {
 	token string
 }
 
-// Client returns a *http.Client, possibly with auth credentials
+// Client returns a *http.Client, possibly with auth credentials.
 func (a *patAuth) Client(ctx context.Context) *http.Client {
 	auth := github.BasicAuthTransport{
 		Username: patUsername,
@@ -223,6 +224,7 @@ func NewClient(ctx context.Context, optFns ...ClientOption) (gitprovider.Client,
 	// the default.
 	var gh *github.Client
 	var domain string
+
 	if opts.Domain == nil || *opts.Domain == defaultDomain {
 		// No domain or the default github.com used
 		domain = defaultDomain

github/client.go

@@ -22,7 +22,7 @@ import (
 	"github.com/fluxcd/go-git-providers/gitprovider"
 )
 
-// ProviderID is the provider ID for GitHub
+// ProviderID is the provider ID for GitHub.
 const ProviderID = gitprovider.ProviderID("github")
 
 func newClient(c *github.Client, domain string, destructiveActions bool) *Client {
@@ -47,9 +47,10 @@ type clientContext struct {
 	destructiveActions bool
 }
 
-// Client implements the gitprovider.Client interface
+// Client implements the gitprovider.Client interface.
 var _ gitprovider.Client = &Client{}
 
+// Client is an interface that allows talking to a Git provider.
 type Client struct {
 	*clientContext
 
@@ -61,13 +62,13 @@ type Client struct {
 // SupportedDomain returns the domain endpoint for this client, e.g. "github.com", "enterprise.github.com" or
 // "my-custom-git-server.com:6443". This allows a higher-level user to know what Client to use for
 // what endpoints.
-// This field is set at client creation time, and can't be changed
+// This field is set at client creation time, and can't be changed.
 func (c *Client) SupportedDomain() string {
 	return c.domain
 }
 
-// ProviderID returns the provider ID "github"
-// This field is set at client creation time, and can't be changed
+// ProviderID returns the provider ID "github".
+// This field is set at client creation time, and can't be changed.
 func (c *Client) ProviderID() gitprovider.ProviderID {
 	return ProviderID
 }

github/client_organization_teams.go

@@ -25,10 +25,10 @@ import (
 	"github.com/fluxcd/go-git-providers/gitprovider"
 )
 
-// TeamsClient implements the gitprovider.TeamsClient interface
+// TeamsClient implements the gitprovider.TeamsClient interface.
 var _ gitprovider.TeamsClient = &TeamsClient{}
 
-// TeamsClient handles teams organization-wide
+// TeamsClient handles teams organization-wide.
 type TeamsClient struct {
 	*clientContext
 	ref gitprovider.OrganizationRef
@@ -50,13 +50,17 @@ func (c *TeamsClient) Get(ctx context.Context, teamName string) (gitprovider.Tea
 		return resp, listErr
 	})
 	if err != nil {
-		return nil, handleHTTPError(err)
+		return nil, err
 	}
 
 	logins := make([]string, 0, len(apiObjs))
 	for _, apiObj := range apiObjs {
-		// TODO: Maybe check for non-empty logins?
-		logins = append(logins, apiObj.GetLogin())
+		// Make sure login isn't nil
+		if apiObj.Login == nil {
+			return nil, fmt.Errorf("didn't expect login to be nil for user: %+v: %w", apiObj, gitprovider.ErrInvalidServerData)
+		}
+
+		logins = append(logins, *apiObj.Login)
 	}
 
 	return &team{
@@ -69,7 +73,7 @@ func (c *TeamsClient) Get(ctx context.Context, teamName string) (gitprovider.Tea
 	}, nil
 }
 
-// List all teams (recursively, in terms of subgroups) within the specific organization
+// List all teams (recursively, in terms of subgroups) within the specific organization.
 //
 // List returns all available organizations, using multiple paginated requests if needed.
 func (c *TeamsClient) List(ctx context.Context) ([]gitprovider.Team, error) {
@@ -83,7 +87,7 @@ func (c *TeamsClient) List(ctx context.Context) ([]gitprovider.Team, error) {
 		return resp, listErr
 	})
 	if err != nil {
-		return nil, handleHTTPError(err)
+		return nil, err
 	}
 
 	// Use .Get() to get detailed information about each member
@@ -99,6 +103,7 @@ func (c *TeamsClient) List(ctx context.Context) ([]gitprovider.Team, error) {
 		if err != nil {
 			return nil, err
 		}
+
 		teams = append(teams, team)
 	}
 

github/client_organizations.go

@@ -25,9 +25,10 @@ import (
 	"github.com/fluxcd/go-git-providers/gitprovider"
 )
 
-// OrganizationsClient implements the gitprovider.OrganizationsClient interface
+// OrganizationsClient implements the gitprovider.OrganizationsClient interface.
 var _ gitprovider.OrganizationsClient = &OrganizationsClient{}
 
+// OrganizationsClient operates on organizations the user has access to.
 type OrganizationsClient struct {
 	*clientContext
 }
@@ -64,7 +65,7 @@ func (c *OrganizationsClient) List(ctx context.Context) ([]gitprovider.Organizat
 		return resp, listErr
 	})
 	if err != nil {
-		return nil, handleHTTPError(err)
+		return nil, err
 	}
 
 	orgs := make([]gitprovider.Organization, 0, len(apiObjs))
@@ -73,6 +74,7 @@ func (c *OrganizationsClient) List(ctx context.Context) ([]gitprovider.Organizat
 		if apiObj.Login == nil {
 			return nil, fmt.Errorf("didn't expect login to be nil for org: %+v: %w", apiObj, gitprovider.ErrInvalidServerData)
 		}
+
 		orgs = append(orgs, newOrganization(c.clientContext, apiObj, gitprovider.OrganizationRef{
 			Domain:       c.domain,
 			Organization: *apiObj.Login,