*: fix lints

Makefile

@@ -68,20 +68,30 @@ static: tools
 	tools/bin/goimports -w -d -format-only -local $(BR_PKG) $$($(PACKAGE_DIRECTORIES)) 2>&1 | $(GOCHECKER)
 	tools/bin/govet --shadow $$($(PACKAGE_DIRECTORIES)) 2>&1 | $(GOCHECKER)
 
+	@# why some lints are disabled?
+	@#   gochecknoglobals - disabled because we do use quite a lot of globals
+	@#          goimports - executed above already
+	@#              gofmt - ditto
+	@#                wsl - too pedantic about the formatting
+	@#             funlen - PENDING REFACTORING
+	@#           gocognit - PENDING REFACTORING
+	@#              godox - TODO
+	@#              gomnd - too many magic numbers, and too pedantic (even 2*x got flagged...)
+	@#        testpackage - several test packages still rely on private functions
+	@#             nestif - PENDING REFACTORING
+	@#           goerr113 - it mistaken pingcap/errors with standard errors
 	CGO_ENABLED=0 tools/bin/golangci-lint run --enable-all --deadline 120s \
 		--disable gochecknoglobals \
-		--disable gochecknoinits \
-		--disable interfacer \
 		--disable goimports \
 		--disable gofmt \
 		--disable wsl \
 		--disable funlen \
-		--disable whitespace \
 		--disable gocognit \
 		--disable godox \
 		--disable gomnd \
 		--disable testpackage \
 		--disable nestif \
+		--disable goerr113 \
 		$$($(PACKAGE_DIRECTORIES))
 
 lint: tools

pkg/backup/client.go

@@ -204,7 +204,6 @@ func appendRanges(tbl *model.TableInfo, tblID int64) ([]kv.KeyRange, error) {
 		kvRanges = append(kvRanges, idxRanges...)
 	}
 	return kvRanges, nil
-
 }
 
 // BuildBackupRangeAndSchema gets the range and schema of tables.

pkg/backup/client_test.go

@@ -96,7 +96,6 @@ func (r *testBackup) TestGetTS(c *C) {
 	ts, err = r.backupClient.GetTS(r.ctx, time.Minute, backupts)
 	c.Assert(err, IsNil)
 	c.Assert(ts, Equals, backupts)
-
 }
 
 func (r *testBackup) TestBuildTableRange(c *C) {
@@ -138,5 +137,4 @@ func (r *testBackup) TestBuildTableRange(c *C) {
 	c.Assert(ranges, DeepEquals, []kv.KeyRange{
 		{StartKey: tablecodec.EncodeRowKey(7, low), EndKey: tablecodec.EncodeRowKey(7, high)},
 	})
-
 }

pkg/backup/metrics.go

@@ -25,7 +25,7 @@ var (
 		})
 )
 
-func init() {
+func init() { // nolint:gochecknoinits
 	prometheus.MustRegister(backupRegionCounters)
 	prometheus.MustRegister(backupRegionHistogram)
 }

pkg/backup/schema.go

@@ -29,7 +29,7 @@ const (
 	DefaultSchemaConcurrency = 64
 )
 
-// Schemas is task for backuping schemas
+// Schemas is task for backuping schemas.
 type Schemas struct {
 	// name -> schema
 	schemas        map[string]backup.Schema

pkg/checksum/executor.go

@@ -241,7 +241,7 @@ func updateChecksumResponse(resp, update *tipb.ChecksumResponse) {
 	resp.TotalBytes += update.TotalBytes
 }
 
-// Executor is a checksum executor
+// Executor is a checksum executor.
 type Executor struct {
 	reqs []*kv.Request
 }

pkg/restore/backoff.go

@@ -13,13 +13,24 @@ import (
 )
 
 var (
-	ErrEpochNotMatch       = errors.NewNoStackError("epoch not match")
-	ErrKeyNotInRegion      = errors.NewNoStackError("key not in region")
+	// ErrEpochNotMatch is the error raised when ingestion failed with "epoch
+	// not match". This error is retryable.
+	ErrEpochNotMatch = errors.NewNoStackError("epoch not match")
+	// ErrKeyNotInRegion is the error raised when ingestion failed with "key not
+	// in region". This error cannot be retried.
+	ErrKeyNotInRegion = errors.NewNoStackError("key not in region")
+	// ErrRewriteRuleNotFound is the error raised when download failed with
+	// "rewrite rule not found". This error cannot be retried
 	ErrRewriteRuleNotFound = errors.NewNoStackError("rewrite rule not found")
-	ErrRangeIsEmpty        = errors.NewNoStackError("range is empty")
-	ErrGRPC                = errors.NewNoStackError("gRPC error")
-	ErrDownloadFailed      = errors.NewNoStackError("download sst failed")
-	ErrIngestFailed        = errors.NewNoStackError("ingest sst failed")
+	// ErrRangeIsEmpty is the error raised when download failed with "range is
+	// empty". This error cannot be retried.
+	ErrRangeIsEmpty = errors.NewNoStackError("range is empty")
+	// ErrGRPC indicates any gRPC communication error. This error can be retried.
+	ErrGRPC = errors.NewNoStackError("gRPC error")
+	// ErrDownloadFailed indicates a generic, non-retryable download error.
+	ErrDownloadFailed = errors.NewNoStackError("download sst failed")
+	// ErrIngestFailed indicates a generic, retryable ingest error.
+	ErrIngestFailed = errors.NewNoStackError("ingest sst failed")
 )
 
 const (

pkg/restore/client.go

@@ -47,7 +47,7 @@ import (
 // checksum tasks.
 const defaultChecksumConcurrency = 64
 
-// Client sends requests to restore files
+// Client sends requests to restore files.
 type Client struct {
 	ctx    context.Context
 	cancel context.CancelFunc
@@ -527,7 +527,6 @@ func (rc *Client) RestoreRaw(startKey []byte, endKey []byte, files []*backup.Fil
 
 	err := rc.fileImporter.SetRawRange(startKey, endKey)
 	if err != nil {
-
 		return errors.Trace(err)
 	}
 

pkg/restore/db.go

@@ -167,16 +167,14 @@ func (db *DB) AlterTiflashReplica(ctx context.Context, table *utils.Table, count
 			zap.Stringer("db", table.Db.Name),
 			zap.Stringer("table", table.Info.Name),
 			zap.Error(err))
-		return err
 	} else if table.TiFlashReplicas > 0 {
 		log.Warn("alter tiflash replica done",
 			zap.Stringer("db", table.Db.Name),
 			zap.Stringer("table", table.Info.Name),
 			zap.Int("originalReplicaCount", table.TiFlashReplicas),
 			zap.Int("replicaCount", count))
-
 	}
-	return nil
+	return err
 }
 
 // Close closes the connection.

pkg/restore/import.go

@@ -27,7 +27,7 @@ import (
 const importScanRegionTime = 10 * time.Second
 const scanRegionPaginationLimit = int(128)
 
-// ImporterClient is used to import a file to TiKV
+// ImporterClient is used to import a file to TiKV.
 type ImporterClient interface {
 	DownloadSST(
 		ctx context.Context,

pkg/restore/split_client.go

@@ -88,7 +88,6 @@ func (c *pdClient) GetStore(ctx context.Context, storeID uint64) (*metapb.Store,
 	}
 	c.storeCache[storeID] = store
 	return store, nil
-
 }
 
 func (c *pdClient) GetRegion(ctx context.Context, key []byte) (*RegionInfo, error) {

pkg/restore/util.go

@@ -166,7 +166,6 @@ func AttachFilesToRanges(
 		rangeTree.Update(rg)
 	}
 	for _, f := range files {
-
 		rg := rangeTree.Find(&rtree.Range{
 			StartKey: f.GetStartKey(),
 			EndKey:   f.GetEndKey(),

pkg/storage/gcs.go

@@ -32,7 +32,6 @@ type GCSBackendOptions struct {
 }
 
 func (options *GCSBackendOptions) apply(gcs *backup.GCS) error {
-
 	gcs.Endpoint = options.Endpoint
 	gcs.StorageClass = options.StorageClass
 	gcs.PredefinedAcl = options.PredefinedACL

pkg/storage/gcs_test.go

@@ -21,7 +21,7 @@ func (r *testStorageSuite) TestGCS(c *C) {
 	server, err := fakestorage.NewServerWithOptions(opts)
 	c.Assert(err, IsNil)
 	bucketName := "testbucket"
-	server.CreateBucket(bucketName)
+	server.CreateBucketWithOpts(fakestorage.CreateBucketOpts{Name: bucketName})
 
 	gcs := &backup.GCS{
 		Bucket:          bucketName,
@@ -65,7 +65,7 @@ func (r *testStorageSuite) TestNewGCSStorage(c *C) {
 	server, err := fakestorage.NewServerWithOptions(opts)
 	c.Assert(err, IsNil)
 	bucketName := "testbucket"
-	server.CreateBucket(bucketName)
+	server.CreateBucketWithOpts(fakestorage.CreateBucketOpts{Name: bucketName})
 
 	{
 		gcs := &backup.GCS{

pkg/storage/local.go

@@ -9,14 +9,15 @@ import (
 	"path"
 )
 
-// localStorage represents local file system storage
+// localStorage represents local file system storage.
 type localStorage struct {
 	base string
 }
 
 func (l *localStorage) Write(ctx context.Context, name string, data []byte) error {
 	filepath := path.Join(l.base, name)
-	return ioutil.WriteFile(filepath, data, 0644)
+	return ioutil.WriteFile(filepath, data, 0644) // nolint:gosec
+	// the backupmeta file _is_ intended to be world-readable.
 }
 
 func (l *localStorage) Read(ctx context.Context, name string) ([]byte, error) {

pkg/storage/parse_test.go

@@ -143,5 +143,4 @@ func (r *testStorageSuite) TestFormatBackendURL(c *C) {
 		},
 	})
 	c.Assert(url.String(), Equals, "gcs://bucket/some%20prefix/")
-
 }

pkg/storage/s3.go

@@ -33,7 +33,7 @@ const (
 	maxRetries = 3
 )
 
-// s3Handlers make it easy to inject test functions
+// s3Handlers make it easy to inject test functions.
 type s3Handlers interface {
 	HeadObjectWithContext(context.Context, *s3.HeadObjectInput, ...request.Option) (*s3.HeadObjectOutput, error)
 	GetObjectWithContext(context.Context, *s3.GetObjectInput, ...request.Option) (*s3.GetObjectOutput, error)
@@ -42,14 +42,14 @@ type s3Handlers interface {
 	WaitUntilObjectExistsWithContext(context.Context, *s3.HeadObjectInput, ...request.WaiterOption) error
 }
 
-// S3Storage info for s3 storage
+// S3Storage info for s3 storage.
 type S3Storage struct {
 	session *session.Session
 	svc     s3Handlers
 	options *backup.S3
 }
 
-// S3BackendOptions contains options for s3 storage
+// S3BackendOptions contains options for s3 storage.
 type S3BackendOptions struct {
 	Endpoint              string `json:"endpoint" toml:"endpoint"`
 	Region                string `json:"region" toml:"region"`
@@ -211,7 +211,7 @@ func newS3Storage( // revive:disable-line:flag-parameter
 	}, nil
 }
 
-// checkBucket checks if a bucket exists
+// checkBucket checks if a bucket exists.
 var checkS3Bucket = func(svc *s3.S3, bucket string) error {
 	input := &s3.HeadBucketInput{
 		Bucket: aws.String(bucket),

pkg/storage/s3_test.go

@@ -99,8 +99,8 @@ func (r *testStorageSuite) TestApplyUpdate(c *C) {
 		s3 := u.GetS3()
 		c.Assert(err, IsNil)
 		c.Assert(s3, DeepEquals, test.s3)
-
 	}
+
 	tests := []testcase{
 		{
 			name: "no region and no endpoint",

pkg/storage/storage.go

@@ -9,7 +9,7 @@ import (
 	"github.com/pingcap/kvproto/pkg/backup"
 )
 
-// ExternalStorage represents a kind of file system storage
+// ExternalStorage represents a kind of file system storage.
 type ExternalStorage interface {
 	// Write file to storage
 	Write(ctx context.Context, name string, data []byte) error

pkg/summary/collector.go

@@ -23,7 +23,7 @@ const (
 	TotalBytes = "total bytes"
 )
 
-// LogCollector collects infos into summary log
+// LogCollector collects infos into summary log.
 type LogCollector interface {
 	SetUnit(unit string)
 

pkg/utils/key_test.go

@@ -30,7 +30,6 @@ func (r *testKeySuite) TestParseKey(c *C) {
 
 	_, err = ParseKey("notSupport", rawKey)
 	c.Assert(err, ErrorMatches, "*unknown format*")
-
 }
 
 func (r *testKeySuite) TestCompareEndKey(c *C) {

pkg/utils/progress.go

@@ -12,7 +12,7 @@ import (
 	"go.uber.org/zap"
 )
 
-// ProgressPrinter prints a progress bar
+// ProgressPrinter prints a progress bar.
 type ProgressPrinter struct {
 	name        string
 	total       int64

pkg/utils/retry.go

@@ -7,10 +7,10 @@ import (
 	"time"
 )
 
-// RetryableFunc presents a retryable opreation
+// RetryableFunc presents a retryable operation.
 type RetryableFunc func() error
 
-// Backoffer implements a backoff policy for retrying operations
+// Backoffer implements a backoff policy for retrying operations.
 type Backoffer interface {
 	// NextBackoff returns a duration to wait before retrying again
 	NextBackoff(err error) time.Duration

pkg/utils/unit.go

@@ -2,11 +2,15 @@
 
 package utils
 
-// unit of storage
 const (
+	// B is number of bytes in one byte.
 	B = uint64(1) << (iota * 10)
+	// KB is number of bytes in one kibibyte.
 	KB
+	// MB is number of bytes in one mebibyte.
 	MB
+	// GB is number of bytes in one gibibyte.
 	GB
+	// TB is number of bytes in one tebibyte.
 	TB
 )

pkg/utils/worker.go

@@ -7,14 +7,14 @@ import (
 	"go.uber.org/zap"
 )
 
-// WorkerPool contains a pool of workers
+// WorkerPool contains a pool of workers.
 type WorkerPool struct {
 	limit   uint
 	workers chan *Worker
 	name    string
 }
 
-// Worker identified by ID
+// Worker identified by ID.
 type Worker struct {
 	ID uint64
 }