golangci: Do not use default excludes

The golangci-lint project has a set of default excludes for linters
which are applied by default. This means that we cannot choose ourselves
which linting messages we want to include, as some of them cannot be
controlled by us.

One noteworthy linting check which is excluded right now checks whether
comments for types and functions are prefixed with their respective
name. This is something which we try to enforce in Gitaly, too, but
naturally there's cases where it slips through review.

So let's disable the default excludes such that we can enable this
linting check. As it turns out, there's quite a lot of cases where we
failed to adhere to this style, which this commit also fixes. One thing
which we don't do though is to have per-package comments, so let's
actively ignore it for now.

22 files changed, 35 insertions, 25 deletions

diff --git a/.golangci.yml b/.golangci.yml
index 93e94f211..64be24ddd 100644
--- a/.golangci.yml
+++ b/.golangci.yml
@@ -29,12 +29,16 @@ linters:
     - varcheck
 
 issues:
+  exclude-use-default: false
   # Excluding configuration per-path, per-linter, per-text and per-source
   exclude-rules:
     # govet checks all struct initializations must be keyed by field names
     - linters:
         - govet
       text: "composite literal uses unkeyed fields"
+    - linters:
+        - stylecheck
+      text: "at least one file in a package should have a package comment"
     - path: "_test.go"
       linters:
         - maligned
diff --git a/client/pool.go b/client/pool.go
index 84067f4ff..d7b2679cd 100644
--- a/client/pool.go
+++ b/client/pool.go
@@ -29,7 +29,7 @@ func NewPool(dialOptions ...grpc.DialOption) *Pool {
 	return NewPoolWithOptions(WithDialOptions(dialOptions...))
 }
 
-// NewPool creates a new connection pool that's ready for use.
+// NewPoolWithOptions creates a new connection pool that's ready for use.
 func NewPoolWithOptions(poolOptions ...PoolOption) *Pool {
 	opts := applyPoolOptions(poolOptions)
 	return &Pool{
diff --git a/internal/git/config.go b/internal/git/config.go
index a63a531f2..fefa597cb 100644
--- a/internal/git/config.go
+++ b/internal/git/config.go
@@ -232,7 +232,7 @@ func (repo RepositoryConfig) parseConfig(data []byte, opts ConfigGetRegexpOpts)
 	return nil, fmt.Errorf("parsing output: %w", err)
 }
 
-// ConfigGetRegexpOpts allows to configure fetching of the configurations using regexp.
+// ConfigUnsetOpts allows to configure fetching of the configurations using regexp.
 type ConfigUnsetOpts struct {
 	// All controls if all values associated with the key needs to be unset.
 	All bool
diff --git a/internal/git/lstree/last_commits.go b/internal/git/lstree/last_commits.go
index d0ef06dc5..def66b5b1 100644
--- a/internal/git/lstree/last_commits.go
+++ b/internal/git/lstree/last_commits.go
@@ -45,7 +45,7 @@ func (e Entries) Swap(i, j int) {
 	e[i], e[j] = e[j], e[i]
 }
 
-// We need to sort in the format [*tree *blobs *submodules]
+// Less sorts entries by type in the order [*tree *blobs *submodules]
 func (e Entries) Less(i, j int) bool {
 	return e[i].Type < e[j].Type
 }
diff --git a/internal/git/staticargs.go b/internal/git/staticargs.go
index c56083805..3242d0b10 100644
--- a/internal/git/staticargs.go
+++ b/internal/git/staticargs.go
@@ -5,7 +5,7 @@ type StaticOption struct {
 	value string
 }
 
-// ValidateArgs just passes through the already trusted value. This never
+// OptionArgs just passes through the already trusted value. This never
 // returns an error.
 func (sa StaticOption) OptionArgs() ([]string, error) { return []string{sa.value}, nil }
 
diff --git a/internal/git/stats/analyzehttp.go b/internal/git/stats/analyzehttp.go
index 7b7303c4b..ed54bed1f 100644
--- a/internal/git/stats/analyzehttp.go
+++ b/internal/git/stats/analyzehttp.go
@@ -340,8 +340,10 @@ const (
 	bandError    = "error"
 )
 
-// These bands map to magic numbers 1, 2, 3. See
+// Bands returns the slice of bands which git uses to transport different kinds
+// of data in a multiplexed way. See
 // https://git-scm.com/docs/protocol-capabilities/2.24.0#_side_band_side_band_64k
+// for more information about the different bands.
 func Bands() []string { return []string{bandPack, bandProgress, bandError} }
 
 func bandToHuman(b byte) (string, error) {
diff --git a/internal/git/stats/packfile_negotiation.go b/internal/git/stats/packfile_negotiation.go
index 18d5dbccc..0f971797c 100644
--- a/internal/git/stats/packfile_negotiation.go
+++ b/internal/git/stats/packfile_negotiation.go
@@ -36,7 +36,8 @@ func ParsePackfileNegotiation(body io.Reader) (PackfileNegotiation, error) {
 	return n, n.Parse(body)
 }
 
-// Expected Format:
+// Parse parses a packfile negotiation. It expects the following format:
+//
 // want <OID> <capabilities\n
 // [want <OID>...]
 // [shallow <OID>]
diff --git a/internal/git2go/conflicts.go b/internal/git2go/conflicts.go
index 67f91808a..9bca079ed 100644
--- a/internal/git2go/conflicts.go
+++ b/internal/git2go/conflicts.go
@@ -70,7 +70,7 @@ func ConflictsCommandFromSerialized(serialized string) (ConflictsCommand, error)
 	return request, nil
 }
 
-// Serialize serializes the conflicts result.
+// SerializeTo serializes the conflicts result and writes it into the writer.
 func (m ConflictsResult) SerializeTo(writer io.Writer) error {
 	return serializeTo(writer, m)
 }
diff --git a/internal/git2go/merge.go b/internal/git2go/merge.go
index 39b2efe15..7659930a1 100644
--- a/internal/git2go/merge.go
+++ b/internal/git2go/merge.go
@@ -64,7 +64,7 @@ func (m MergeResult) SerializeTo(w io.Writer) error {
 	return serializeTo(w, m)
 }
 
-// Merge performs a merge via gitaly-git2go.
+// Run performs a merge via gitaly-git2go.
 func (m MergeCommand) Run(ctx context.Context, cfg config.Cfg) (MergeResult, error) {
 	if err := m.verify(); err != nil {
 		return MergeResult{}, fmt.Errorf("merge: %w: %s", ErrInvalidArgument, err.Error())
diff --git a/internal/log/hook.go b/internal/log/hook.go
index 1aec78efb..80f0a4441 100644
--- a/internal/log/hook.go
+++ b/internal/log/hook.go
@@ -45,7 +45,7 @@ func (h *HookLogger) Fatalf(format string, a ...interface{}) {
 	h.logger.Fatalf(format, a...)
 }
 
-// Fatalf logs a formatted error at the Fatal level
+// Errorf logs a formatted error at the Fatal level
 func (h *HookLogger) Errorf(format string, a ...interface{}) {
 	h.logger.Errorf(format, a...)
 }
diff --git a/internal/praefect/coordinator.go b/internal/praefect/coordinator.go
index 4d5d7b6c9..9d7e0cf59 100644
--- a/internal/praefect/coordinator.go
+++ b/internal/praefect/coordinator.go
@@ -448,7 +448,7 @@ func (c *Coordinator) mutatorStreamParameters(ctx context.Context, call grpcCall
 	return proxy.NewStreamParameters(primaryDest, secondaryDests, reqFinalizer, nil), nil
 }
 
-// streamDirector determines which downstream servers receive requests
+// StreamDirector determines which downstream servers receive requests
 func (c *Coordinator) StreamDirector(ctx context.Context, fullMethodName string, peeker proxy.StreamPeeker) (*proxy.StreamParameters, error) {
 	// For phase 1, we need to route messages based on the storage location
 	// to the appropriate Gitaly node.
diff --git a/internal/praefect/datastore/assignment.go b/internal/praefect/datastore/assignment.go
index 77904c50c..887646f55 100644
--- a/internal/praefect/datastore/assignment.go
+++ b/internal/praefect/datastore/assignment.go
@@ -30,7 +30,7 @@ type AssignmentStore struct {
 	configuredStorages map[string][]string
 }
 
-// NewAssignmentsStore returns a new AssignmentStore using the passed in database.
+// NewAssignmentStore returns a new AssignmentStore using the passed in database.
 func NewAssignmentStore(db glsql.Querier, configuredStorages map[string][]string) AssignmentStore {
 	return AssignmentStore{db: db, configuredStorages: configuredStorages}
 }
diff --git a/internal/praefect/datastore/repository_store.go b/internal/praefect/datastore/repository_store.go
index 288a4655d..488cbe5cd 100644
--- a/internal/praefect/datastore/repository_store.go
+++ b/internal/praefect/datastore/repository_store.go
@@ -109,7 +109,7 @@ type PostgresRepositoryStore struct {
 	storages
 }
 
-// NewLocalRepositoryStore returns a Postgres implementation of RepositoryStore.
+// NewPostgresRepositoryStore returns a Postgres implementation of RepositoryStore.
 func NewPostgresRepositoryStore(db glsql.Querier, configuredStorages map[string][]string) *PostgresRepositoryStore {
 	return &PostgresRepositoryStore{db: db, storages: storages(configuredStorages)}
 }
diff --git a/internal/praefect/grpc-proxy/proxy/codec.go b/internal/praefect/grpc-proxy/proxy/codec.go
index 77deb6daa..ef56ff2e3 100644
--- a/internal/praefect/grpc-proxy/proxy/codec.go
+++ b/internal/praefect/grpc-proxy/proxy/codec.go
@@ -7,8 +7,8 @@ import (
 	"google.golang.org/grpc/encoding"
 )
 
-// This interface is required to maintain compatibility with both grpc.Codec
-// and encoding.Codec.
+// Codec is an interface which is required to maintain compatibility with both
+// grpc.Codec and encoding.Codec.
 type Codec interface {
 	Marshal(v interface{}) ([]byte, error)
 	Unmarshal(data []byte, v interface{}) error
@@ -16,7 +16,7 @@ type Codec interface {
 	String() string
 }
 
-// Codec returns a proxying encoding.Codec with the default protobuf codec as parent.
+// NewCodec returns a proxying encoding.Codec with the default protobuf codec as parent.
 //
 // See CodecWithParent.
 func NewCodec() Codec {
diff --git a/internal/praefect/grpc-proxy/proxy/peeker.go b/internal/praefect/grpc-proxy/proxy/peeker.go
index 459468dbe..6825d7f37 100644
--- a/internal/praefect/grpc-proxy/proxy/peeker.go
+++ b/internal/praefect/grpc-proxy/proxy/peeker.go
@@ -6,7 +6,7 @@ import (
 	"google.golang.org/grpc"
 )
 
-// StreamModifier abstracts away the gRPC stream being forwarded so that it can
+// StreamPeeker abstracts away the gRPC stream being forwarded so that it can
 // be inspected and modified.
 type StreamPeeker interface {
 	// Peek allows a director to peek one message into the request stream without
diff --git a/internal/praefect/router.go b/internal/praefect/router.go
index d4b08989c..2f4a9848f 100644
--- a/internal/praefect/router.go
+++ b/internal/praefect/router.go
@@ -23,7 +23,7 @@ type StorageMutatorRoute struct {
 	Secondaries []RouterNode
 }
 
-// StorageMutatorRoute describes how to route a repository scoped mutator call.
+// RepositoryMutatorRoute describes how to route a repository scoped mutator call.
 type RepositoryMutatorRoute struct {
 	// Primary is the primary node of the transaction.
 	Primary RouterNode
diff --git a/internal/praefect/router_node_manager.go b/internal/praefect/router_node_manager.go
index 6bc850ca6..f26433700 100644
--- a/internal/praefect/router_node_manager.go
+++ b/internal/praefect/router_node_manager.go
@@ -28,7 +28,7 @@ func toRouterNodes(nodes []nodes.Node) []RouterNode {
 	return out
 }
 
-// NeWNodeManagerRouter returns a router that uses the NodeManager to make routing decisions.
+// NewNodeManagerRouter returns a router that uses the NodeManager to make routing decisions.
 func NewNodeManagerRouter(mgr nodes.Manager, rs datastore.RepositoryStore) Router {
 	return &nodeManagerRouter{mgr: mgr, rs: rs}
 }
diff --git a/internal/praefect/router_per_repository.go b/internal/praefect/router_per_repository.go
index d2174c9bb..9f88318b6 100644
--- a/internal/praefect/router_per_repository.go
+++ b/internal/praefect/router_per_repository.go
@@ -107,8 +107,10 @@ func (r *PerRepositoryRouter) pickRandom(nodes []RouterNode) (RouterNode, error)
 	return nodes[r.rand.Intn(len(nodes))], nil
 }
 
-// The only storage scoped accessor RPC is RemoteService/FindRemoteRepository, which in turn executes a command
-// without a repository. This can be done by any Gitaly server as it doesn't depend on the state on the server.
+// RouteStorageAccessor routes requests for storage-scoped accessor RPCs. The
+// only storage scoped accessor RPC is RemoteService/FindRemoteRepository,
+// which in turn executes a command without a repository. This can be done by
+// any Gitaly server as it doesn't depend on the state on the server.
 func (r *PerRepositoryRouter) RouteStorageAccessor(ctx context.Context, virtualStorage string) (RouterNode, error) {
 	healthyNodes, err := r.healthyNodes(virtualStorage)
 	if err != nil {
diff --git a/internal/protoutil/extension.go b/internal/protoutil/extension.go
index 34e54d504..93a032feb 100644
--- a/internal/protoutil/extension.go
+++ b/internal/protoutil/extension.go
@@ -39,7 +39,7 @@ func GetTargetRepositoryExtension(m *descriptor.FieldDescriptorProto) (bool, err
 	return getBoolExtension(m.GetOptions(), gitalypb.E_TargetRepository)
 }
 
-// GetAdditionaRepositoryExtension gets the target_repository extension from a field descriptor
+// GetAdditionalRepositoryExtension gets the target_repository extension from a field descriptor
 func GetAdditionalRepositoryExtension(m *descriptor.FieldDescriptorProto) (bool, error) {
 	return getBoolExtension(m.GetOptions(), gitalypb.E_AdditionalRepository)
 }
diff --git a/internal/testhelper/git_hooks.go b/internal/testhelper/git_hooks.go
index 02d3cb65d..b6fa75973 100644
--- a/internal/testhelper/git_hooks.go
+++ b/internal/testhelper/git_hooks.go
@@ -13,7 +13,7 @@ import (
 	"gitlab.com/gitlab-org/gitaly/internal/gitaly/config"
 )
 
-// dump the env vars that the custom hooks receives to a file
+// WriteEnvToCustomHook dumps the env vars that the custom hooks receives to a file
 func WriteEnvToCustomHook(t testing.TB, repoPath, hookName string) (string, func()) {
 	hookOutputTemp, err := ioutil.TempFile("", "")
 	require.NoError(t, err)
@@ -49,7 +49,7 @@ end
 	return cleanup
 }
 
-// write a hook in the repo/path.git/custom_hooks directory
+// WriteCustomHook writes a hook in the repo/path.git/custom_hooks directory
 func WriteCustomHook(repoPath, name string, content []byte) (func(), error) {
 	fullPath := filepath.Join(repoPath, "custom_hooks", name)
 	return WriteExecutable(fullPath, content)
diff --git a/internal/testhelper/test_hook.go b/internal/testhelper/test_hook.go
index 8d0b0264c..85fe4d244 100644
--- a/internal/testhelper/test_hook.go
+++ b/internal/testhelper/test_hook.go
@@ -18,7 +18,7 @@ func DiscardTestLogger(tb testing.TB) *log.Logger {
 	return logger
 }
 
-// DiscardTestLogger created a logrus entry that discards everything.
+// DiscardTestEntry creates a logrus entry that discards everything.
 func DiscardTestEntry(tb testing.TB) *log.Entry {
 	return log.NewEntry(DiscardTestLogger(tb))
 }
diff --git a/internal/testhelper/testserver.go b/internal/testhelper/testserver.go
index f2230f96c..61f66d508 100644
--- a/internal/testhelper/testserver.go
+++ b/internal/testhelper/testserver.go
@@ -57,7 +57,8 @@ func WithStorages(storages []string) TestServerOpt {
 	}
 }
 
-// WithStorages is a TestServerOpt that sets the storages for a TestServer
+// WithInternalSocket is a TestServerOpt that will cause the TestServer to
+// listen on its internal socket.
 func WithInternalSocket(cfg config.Cfg) TestServerOpt {
 	return func(t *TestServer) {
 		t.withInternalSocketPath = cfg.GitalyInternalSocketPath()