Merge branch '782-lfs-lint' into 'main'

Lint fixes for lfs packages

Closes #782

See merge request https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/1139



Merged-by: Ash McKenzie <amckenzie@gitlab.com>
Approved-by: Patrick Bajao <ebajao@gitlab.com>
Co-authored-by: Archish <archishthakkar@gmail.com>

internal/gitlabnet/lfsauthenticate/client_test.go

@@ -7,6 +7,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"gitlab.com/gitlab-org/gitlab-shell/v14/client/testserver"
@@ -26,10 +27,10 @@ func setup(t *testing.T) []testserver.TestRequestHandler {
 			Handler: func(w http.ResponseWriter, r *http.Request) {
 				b, err := io.ReadAll(r.Body)
 				defer r.Body.Close()
-				require.NoError(t, err)
+				assert.NoError(t, err)
 
 				var request *Request
-				require.NoError(t, json.Unmarshal(b, &request))
+				assert.NoError(t, json.Unmarshal(b, &request))
 
 				switch request.KeyID {
 				case keyID:
@@ -39,7 +40,7 @@ func setup(t *testing.T) []testserver.TestRequestHandler {
 						"repository_http_path": "https://gitlab.com/repo/path",
 						"expires_in":           1800,
 					}
-					require.NoError(t, json.NewEncoder(w).Encode(body))
+					assert.NoError(t, json.NewEncoder(w).Encode(body))
 				case "forbidden":
 					w.WriteHeader(http.StatusForbidden)
 				case "broken":
@@ -88,7 +89,7 @@ func TestFailedRequests(t *testing.T) {
 			_, err = client.Authenticate(context.Background(), operation, repo, "")
 			require.Error(t, err)
 
-			require.Equal(t, tc.expectedOutput, err.Error())
+			assert.Equal(t, tc.expectedOutput, err.Error())
 		})
 	}
 }
@@ -128,7 +129,7 @@ func TestSuccessfulRequests(t *testing.T) {
 				ExpiresIn: 1800,
 			}
 
-			require.Equal(t, expectedResponse, response)
+			assert.Equal(t, expectedResponse, response)
 		})
 	}
 }

internal/gitlabnet/lfstransfer/client.go

+// Package lfstransfer provides functionality for handling LFS (Large File Storage) transfers.
 package lfstransfer
 
 import (
@@ -17,6 +18,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-shell/v14/internal/gitlabnet"
 )
 
+// Client holds configuration, arguments, and authentication details for the client.
 type Client struct {
 	config *config.Config
 	args   *commandargs.Shell
@@ -25,6 +27,7 @@ type Client struct {
 	header string
 }
 
+// BatchAction represents an action for a batch operation with metadata.
 type BatchAction struct {
 	Href      string            `json:"href"`
 	Header    map[string]string `json:"header,omitempty"`
@@ -32,6 +35,7 @@ type BatchAction struct {
 	ExpiresIn int               `json:"expires_in,omitempty"`
 }
 
+// BatchObject represents an object in a batch operation with its metadata and actions.
 type BatchObject struct {
 	Oid           string                  `json:"oid,omitempty"`
 	Size          int64                   `json:"size"`
@@ -50,6 +54,7 @@ type batchRequest struct {
 	HashAlgorithm string         `json:"hash_algo,omitempty"`
 }
 
+// BatchResponse contains batch operation results and the hash algorithm used.
 type BatchResponse struct {
 	Objects       []*BatchObject `json:"objects"`
 	HashAlgorithm string         `json:"hash_algo,omitempty"`
@@ -79,10 +84,12 @@ type listLocksVerifyRequest struct {
 	Ref    *batchRef `json:"ref,omitempty"`
 }
 
+// LockOwner represents the owner of a lock.
 type LockOwner struct {
 	Name string `json:"name"`
 }
 
+// Lock represents a lock with its ID, path, timestamp, and owner details.
 type Lock struct {
 	ID       string     `json:"id"`
 	Path     string     `json:"path"`
@@ -90,19 +97,23 @@ type Lock struct {
 	Owner    *LockOwner `json:"owner"`
 }
 
+// ListLocksResponse contains a list of locks and a cursor for pagination.
 type ListLocksResponse struct {
 	Locks      []*Lock `json:"locks,omitempty"`
 	NextCursor string  `json:"next_cursor,omitempty"`
 }
 
+// ListLocksVerifyResponse provides lists of locks for "ours" and "theirs" with a cursor for pagination.
 type ListLocksVerifyResponse struct {
 	Ours       []*Lock `json:"ours,omitempty"`
 	Theirs     []*Lock `json:"theirs,omitempty"`
 	NextCursor string  `json:"next_cursor,omitempty"`
 }
 
+// ClientHeader specifies the content type for Git LFS JSON requests.
 var ClientHeader = "application/vnd.git-lfs+json"
 
+// NewClient creates a new Client instance using the provided configuration and credentials.
 func NewClient(config *config.Config, args *commandargs.Shell, href string, auth string) (*Client, error) {
 	return &Client{config: config, args: args, href: href, auth: auth, header: ClientHeader}, nil
 }
@@ -121,6 +132,7 @@ func newHTTPClient() *retryablehttp.Client {
 	return client
 }
 
+// Batch performs a batch operation on objects and returns the result.
 func (c *Client) Batch(operation string, reqObjects []*BatchObject, ref string, reqHashAlgo string) (*BatchResponse, error) {
 	// FIXME: This causes tests to fail
 	// if ref == "" {
@@ -211,6 +223,7 @@ func (c *Client) PutObject(_, href string, headers map[string]string, r io.Reade
 	return nil
 }
 
+// Lock acquires a lock for the specified path with an optional reference name.
 func (c *Client) Lock(path, refname string) (*Lock, error) {
 	var ref *batchRef
 	if refname != "" {
@@ -268,6 +281,7 @@ func (c *Client) Lock(path, refname string) (*Lock, error) {
 	}
 }
 
+// Unlock releases the lock with the given id, optionally forcing the unlock.
 func (c *Client) Unlock(id string, force bool, refname string) (*Lock, error) {
 	var ref *batchRef
 	if refname != "" {
@@ -318,6 +332,7 @@ func (c *Client) Unlock(id string, force bool, refname string) (*Lock, error) {
 	}
 }
 
+// ListLocksVerify retrieves locks for the given path and id, with optional pagination.
 func (c *Client) ListLocksVerify(path, id, cursor string, limit int, ref string) (*ListLocksVerifyResponse, error) {
 	url, err := url.Parse(c.href)
 	if err != nil {

support/lint_last_known_acceptable.txt

@@ -247,25 +247,7 @@ internal/gitlabnet/client.go:21:15: ST1005: error strings should not be capitali
 internal/gitlabnet/client.go:27:1: exported: exported function ParseJSON should have comment or be unexported (revive)
 internal/gitlabnet/client.go:35:1: exported: exported function ParseIP should have comment or be unexported (revive)
 internal/gitlabnet/healthcheck/client_test.go:19:41: unused-parameter: parameter 'r' seems to be unused, consider removing or renaming it as _ (revive)
-internal/gitlabnet/lfsauthenticate/client_test.go:29:5: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/lfsauthenticate/client_test.go:32:5: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/lfsauthenticate/client_test.go:42:6: go-require: do not use require in http handlers (testifylint)
-internal/gitlabnet/lfstransfer/client.go:1:1: package-comments: should have a package comment (revive)
-internal/gitlabnet/lfstransfer/client.go:20:6: exported: exported type Client should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:28:6: exported: exported type BatchAction should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:35:6: exported: exported type BatchObject should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:53:6: exported: exported type BatchResponse should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:82:6: exported: exported type LockOwner should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:86:6: exported: exported type Lock should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:93:6: exported: exported type ListLocksResponse should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:98:6: exported: exported type ListLocksVerifyResponse should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:104:5: exported: exported var ClientHeader should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:106:1: exported: exported function NewClient should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:124:1: exported: exported method Client.Batch should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:125: internal/gitlabnet/lfstransfer/client.go:125: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "FIXME: This causes tests to fail" (godox)
-internal/gitlabnet/lfstransfer/client.go:214:1: exported: exported method Client.Lock should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:271:1: exported: exported method Client.Unlock should have comment or be unexported (revive)
-internal/gitlabnet/lfstransfer/client.go:321:1: exported: exported method Client.ListLocksVerify should have comment or be unexported (revive)
+internal/gitlabnet/lfstransfer/client.go:137: internal/gitlabnet/lfstransfer/client.go:137: Line contains TODO/BUG/FIXME/NOTE/OPTIMIZE/HACK: "FIXME: This causes tests to fail" (godox)
 internal/gitlabnet/personalaccesstoken/client_test.go:30:5: go-require: do not use require in http handlers (testifylint)
 internal/sshd/gssapi_unsupported.go:11:1: exported: exported function NewGSSAPIServer should have comment or be unexported (revive)
 internal/sshd/gssapi_unsupported.go:19:6: exported: exported type OSGSSAPIServer should have comment or be unexported (revive)