Enforce leading space on comments

Makefile

@@ -361,6 +361,12 @@ $(BUILD)/gomod-lint: go.mod internal/tools/go.mod common/archiver/gcloud/go.mod
 $(BUILD)/code-lint: $(LINT_SRC) $(BIN)/revive | $(BUILD)
 	$Q echo "lint..."
 	$Q $(BIN)/revive -config revive.toml -exclude './vendor/...' -exclude './.gen/...' -formatter stylish ./...
+	$Q bad="$$(find . -type f -name '*.go' -not -path './idls/*' | xargs grep -n -E '^\s*//\S' | grep -E -v '^[^:]+:[^:]+:\s*//[a-z]+:[a-z]+' || true)"; \
+		if [ -n "$$bad" ]; then \
+		  echo "$$bad" >&2; \
+		  echo 'non-directive comments must have a space after the "//"' >&2; \
+		  exit 1; \
+		fi
 	$Q touch $@
 
 # fmt and copyright are mutually cyclic with their inputs, so if a copyright header is modified:

canary/config.go

@@ -76,7 +76,7 @@ type (
 	// Cron contains configuration for the cron workflow for canary
 	Cron struct {
 		CronSchedule         string        `yaml:"cronSchedule"`         // default to "@every 30s"
-		CronExecutionTimeout time.Duration `yaml:"cronExecutionTimeout"` //default to 18 minutes
+		CronExecutionTimeout time.Duration `yaml:"cronExecutionTimeout"` // default to 18 minutes
 		StartJobTimeout      time.Duration `yaml:"startJobTimeout"`      // default to 9 minutes
 	}
 

common/archiver/s3store/historyArchiver_test.go

@@ -657,8 +657,8 @@ func (s *historyArchiverSuite) TestArchiveAndGet() {
 }
 
 func (s *historyArchiverSuite) newTestHistoryArchiver(historyIterator archiver.HistoryIterator) *historyArchiver {
-	//config := &config.S3Archiver{}
-	//archiver, err := newHistoryArchiver(s.container, config, historyIterator)
+	// config := &config.S3Archiver{}
+	// archiver, err := newHistoryArchiver(s.container, config, historyIterator)
 	archiver := &historyArchiver{
 		container:       s.container,
 		s3cli:           s.s3cli,

common/backoff/retrypolicy_test.go

@@ -188,7 +188,7 @@ func (s *RetryPolicySuite) TestNoMaxAttempts() {
 	r, clock := createRetrier(policy)
 	for i := 0; i < 100; i++ {
 		next := r.NextBackOff()
-		//print("Iter: ", i, ", Next Backoff: ", next.String(), "\n")
+		// print("Iter: ", i, ", Next Backoff: ", next.String(), "\n")
 		s.True(next > 0 || next == done, "Unexpected value for next retry duration: %v", next)
 		clock.moveClock(next)
 	}
@@ -200,7 +200,7 @@ func (s *RetryPolicySuite) TestUnbounded() {
 	r, clock := createRetrier(policy)
 	for i := 0; i < 100; i++ {
 		next := r.NextBackOff()
-		//print("Iter: ", i, ", Next Backoff: ", next.String(), "\n")
+		// print("Iter: ", i, ", Next Backoff: ", next.String(), "\n")
 		s.True(next > 0 || next == done, "Unexpected value for next retry duration: %v", next)
 		clock.moveClock(next)
 	}

common/config/metrics.go

@@ -126,7 +126,7 @@ func (c *Metrics) newStatsdScope(logger log.Logger) tally.Scope {
 	if err != nil {
 		logger.Fatal("error creating statsd client", tag.Error(err))
 	}
-	//NOTE: according to ( https://github.com/uber-go/tally )Tally's statsd implementation doesn't support tagging.
+	// NOTE: according to ( https://github.com/uber-go/tally )Tally's statsd implementation doesn't support tagging.
 	// Therefore, we implement Tally interface to have a statsd reporter that can support tagging
 	reporter := statsdreporter.NewReporter(statter, tallystatsdreporter.Options{})
 	scopeOpts := tally.ScopeOptions{

common/config/tls.go

@@ -42,7 +42,7 @@ type (
 		CertFile string `yaml:"certFile"`
 		KeyFile  string `yaml:"keyFile"`
 
-		CaFile  string   `yaml:"caFile"` //optional depending on server config
+		CaFile  string   `yaml:"caFile"` // optional depending on server config
 		CaFiles []string `yaml:"caFiles"`
 		// If you want to verify the hostname and server cert (like a wildcard for cass cluster) then you should turn this on
 		// This option is basically the inverse of InSecureSkipVerify

common/domain/replication_queue.go

@@ -208,7 +208,7 @@ func (q *replicationQueueImpl) GetMessagesFromDLQ(
 			return nil, nil, fmt.Errorf("failed to decode dlq task: %v", err)
 		}
 
-		//Overwrite to local cluster message id
+		// Overwrite to local cluster message id
 		replicationTask.SourceTaskId = common.Int64Ptr(int64(message.ID))
 		replicationTasks = append(replicationTasks, thrift.ToReplicationTask(&replicationTask))
 	}

common/dynamicconfig/configstore/config/config.go

@@ -22,8 +22,8 @@ package config
 
 import "time"
 
-//This package is necessary to avoid import cycle as config_store_client imports common/config
-//while common/config imports this ClientConfig definition
+// This package is necessary to avoid import cycle as config_store_client imports common/config
+// while common/config imports this ClientConfig definition
 
 // ClientConfig is the config for the config store based dynamic config client.
 // It specifies how often the cached config should be updated by checking underlying database.

common/dynamicconfig/configstore/config_store_client.go

@@ -296,8 +296,8 @@ func (csc *configStoreClient) UpdateValue(name dc.Key, value interface{}) error
 }
 
 func (csc *configStoreClient) RestoreValue(name dc.Key, filters map[dc.Filter]interface{}) error {
-	//if empty filter provided, update fallback value.
-	//if u want to remove entire entry, just do update value with empty
+	// if empty filter provided, update fallback value.
+	// if u want to remove entire entry, just do update value with empty
 	loaded := csc.values.Load()
 	if loaded == nil {
 		return dc.NotFoundError
@@ -345,20 +345,20 @@ func (csc *configStoreClient) ListValue(name dc.Key) ([]*types.DynamicConfigEntr
 	}
 	listAll := false
 	if name == nil {
-		//if key is not specified, return all entries
+		// if key is not specified, return all entries
 		listAll = true
 	} else if _, ok := currentCached.dcEntries[name.String()]; !ok {
-		//if key is not known, return all entries
+		// if key is not known, return all entries
 		listAll = true
 	}
 	if listAll {
-		//if key is not known/specified, return all entries
+		// if key is not known/specified, return all entries
 		resList = make([]*types.DynamicConfigEntry, 0, len(currentCached.dcEntries))
 		for _, entry := range currentCached.dcEntries {
 			resList = append(resList, copyDynamicConfigEntry(entry))
 		}
 	} else {
-		//if key is known, return just that specific entry
+		// if key is known, return just that specific entry
 		resList = make([]*types.DynamicConfigEntry, 0, 1)
 		resList = append(resList, currentCached.dcEntries[name.String()])
 	}
@@ -385,9 +385,9 @@ func (csc *configStoreClient) Start() {
 }
 
 func (csc *configStoreClient) updateValue(name dc.Key, dcValues []*types.DynamicConfigValue, retryAttempts int) error {
-	//since values are not unique, no way to know if you are trying to update a specific value
-	//or if you want to add another of the same value with different filters.
-	//UpdateValue will replace everything associated with dc key.
+	// since values are not unique, no way to know if you are trying to update a specific value
+	// or if you want to add another of the same value with different filters.
+	// UpdateValue will replace everything associated with dc key.
 	for _, dcValue := range dcValues {
 		if err := validateKeyDataBlobPair(name, dcValue.Value); err != nil {
 			return err
@@ -465,12 +465,12 @@ func (csc *configStoreClient) updateValue(name dc.Key, dcValues []*types.Dynamic
 
 	select {
 	case <-ctx.Done():
-		//potentially we can retry on timeout
+		// potentially we can retry on timeout
 		return errors.New("timeout error on update")
 	default:
 		if err != nil {
 			if _, ok := err.(*persistence.ConditionFailedError); ok && retryAttempts > 0 {
-				//fetch new config and retry
+				// fetch new config and retry
 				err := csc.update()
 				if err != nil {
 					return err
@@ -575,7 +575,7 @@ func (csc *configStoreClient) update() error {
 }
 
 func (csc *configStoreClient) storeValues(snapshot *persistence.DynamicConfigSnapshot) error {
-	//Converting the list of dynamic config entries into a map for better lookup performance
+	// Converting the list of dynamic config entries into a map for better lookup performance
 	var dcEntryMap map[string]*types.DynamicConfigEntry
 	if snapshot.Values.Entries == nil {
 		dcEntryMap = nil

common/dynamicconfig/constants_test.go

@@ -40,7 +40,7 @@ func TestConstantSuite(t *testing.T) {
 }
 
 func (s *constantSuite) TestListAllProductionKeys() {
-	//check if we given enough capacity
+	// check if we given enough capacity
 	testResult := ListAllProductionKeys()
 	s.GreaterOrEqual(len(IntKeys)+len(BoolKeys)+len(FloatKeys)+len(StringKeys)+len(DurationKeys)+len(MapKeys), len(testResult))
 	s.Equal(TestGetIntPropertyFilteredByTaskListInfoKey+1, testResult[0])

common/elasticsearch/client/v6/client_bulk.go

@@ -80,8 +80,8 @@ func (v *v6BulkProcessor) Add(request *bulk.GenericBulkableAddRequest) {
 			Version(request.Version).
 			Doc(request.Doc)
 	case bulk.BulkableCreateRequest:
-		//for bulk create request still calls the bulk index method
-		//with providing operation type
+		// for bulk create request still calls the bulk index method
+		// with providing operation type
 		req = elastic.NewBulkIndexRequest().
 			OpType("create").
 			Index(request.Index).

common/elasticsearch/client/v7/client_bulk.go

@@ -76,8 +76,8 @@ func (v *v7BulkProcessor) Add(request *bulk.GenericBulkableAddRequest) {
 			Version(request.Version).
 			Doc(request.Doc)
 	case bulk.BulkableCreateRequest:
-		//for bulk create request still calls the bulk index method
-		//with providing operation type
+		// for bulk create request still calls the bulk index method
+		// with providing operation type
 		req = elastic.NewBulkIndexRequest().
 			OpType("create").
 			Index(request.Index).

common/elasticsearch/esql/cadencesql.go

@@ -63,7 +63,7 @@ func (e *ESql) ConvertCadence(sql string, domainID string, pagination ...interfa
 		return "", nil, err
 	}
 
-	//sql valid, start to handle
+	// sql valid, start to handle
 	switch stmt := stmt.(type) {
 	case *sqlparser.Select:
 		dsl, sortFields, err = e.convertSelect(*(stmt), domainID, pagination...)

common/elasticsearch/esql/esql.go

@@ -136,7 +136,7 @@ func (e *ESql) Convert(sql string, pagination ...interface{}) (dsl string, sortF
 		return "", nil, err
 	}
 
-	//sql valid, start to handle
+	// sql valid, start to handle
 	switch stmt := stmt.(type) {
 	case *sqlparser.Select:
 		dsl, sortField, err = e.convertSelect(*(stmt), "", pagination...)

common/log/tag/interface.go

@@ -69,7 +69,7 @@ func newBoolTag(key string, value bool) Tag {
 }
 
 func newErrorTag(key string, value error) Tag {
-	//NOTE zap already chosen "error" as key
+	// NOTE zap already chosen "error" as key
 	return Tag{
 		field: zap.Error(value),
 	}

common/log/tag/tags.go

@@ -34,7 +34,7 @@ import (
 //   1. Workflow: these tags are information that are useful to our customer, like workflow-id/run-id/task-list/...
 //   2. System : these tags are internal information which usually cannot be understood by our customers,
 
-///////////////////  Common tags defined here ///////////////////
+// /////////////////  Common tags defined here ///////////////////
 
 // Error returns tag for Error
 func Error(err error) Tag {
@@ -59,7 +59,7 @@ func LatestTime(time int64) Tag {
 	return newInt64("latest-time", time)
 }
 
-///////////////////  Workflow tags defined here: ( wf is short for workflow) ///////////////////
+// /////////////////  Workflow tags defined here: ( wf is short for workflow) ///////////////////
 
 // WorkflowAction returns tag for WorkflowAction
 func workflowAction(action string) Tag {
@@ -342,7 +342,7 @@ func WorkflowEventType(eventType string) Tag {
 	return newStringTag("wf-event-type", eventType)
 }
 
-///////////////////  System tags defined here:  ///////////////////
+// /////////////////  System tags defined here:  ///////////////////
 // Tags with pre-define values
 
 // component returns tag for component
@@ -753,7 +753,7 @@ func TokenLastEventID(id int64) Tag {
 	return newInt64("token-last-event-id", id)
 }
 
-///////////////////  XDC tags defined here: xdc- ///////////////////
+// /////////////////  XDC tags defined here: xdc- ///////////////////
 
 // SourceCluster returns tag for SourceCluster
 func SourceCluster(sourceCluster string) Tag {
@@ -824,7 +824,7 @@ func ResponseMaxSize(size int) Tag {
 	return newInt("response-max-size", size)
 }
 
-///////////////////  Archival tags defined here: archival- ///////////////////
+// /////////////////  Archival tags defined here: archival- ///////////////////
 // archival request tags
 
 // ArchivalCallerServiceName returns tag for the service name calling archival client

common/membership/resolver.go

@@ -54,7 +54,7 @@ type (
 		// EvictSelf evicts this member from the membership ring. After this method is
 		// called, other members should discover that this node is no longer part of the
 		// ring.
-		//This primitive is useful to carry out graceful host shutdown during deployments.
+		// This primitive is useful to carry out graceful host shutdown during deployments.
 		EvictSelf() error
 
 		// Lookup will return host which is an owner for provided key.

common/messaging/kafka/partition_ack_manager.go

@@ -34,7 +34,7 @@ import (
 // assuming reading messages is in order and continuous(no skipping)
 type partitionAckManager struct {
 	sync.RWMutex
-	ackMgrs map[int32]messaging.AckManager //map from partition to its ackManager
+	ackMgrs map[int32]messaging.AckManager // map from partition to its ackManager
 	scopes  map[int32]metrics.Scope        // map from partition to its Scope
 
 	metricsClient metrics.Client

common/metrics/client.go

@@ -28,7 +28,7 @@ import (
 
 // ClientImpl is used for reporting metrics by various Cadence services
 type ClientImpl struct {
-	//parentReporter is the parent scope for the metrics
+	// parentReporter is the parent scope for the metrics
 	parentScope tally.Scope
 	childScopes map[int]tally.Scope
 	metricDefs  map[int]metricDefinition

common/metrics/defs.go

@@ -36,7 +36,6 @@ type (
 
 	// metricDefinition contains the definition for a metric
 	metricDefinition struct {
-		//nolint
 		metricType       MetricType    // metric type
 		metricName       MetricName    // metric name
 		metricRollupName MetricName    // optional. if non-empty, this name must be used for rolled-up version of this metric

common/metrics/tally/statsd/reporter.go

@@ -31,7 +31,7 @@ import (
 )
 
 type cadenceTallyStatsdReporter struct {
-	//Wrapper on top of "github.com/uber-go/tally/statsd"
+	// Wrapper on top of "github.com/uber-go/tally/statsd"
 	tallystatsd tally.StatsReporter
 }
 

common/metrics/tally/statsd/reporter_test.go

@@ -51,7 +51,7 @@ func TestMetricNameWithTagsStability(t *testing.T) {
 	}
 	name := "test-metric-name2"
 
-	//test the order is stable
+	// test the order is stable
 	for i := 1; i <= 16; i++ {
 		assert.Equal(t, r.metricNameWithTags(name, tags), r.metricNameWithTags(name, tags))
 	}

common/peerprovider/ringpopprovider/config_test.go

@@ -220,22 +220,22 @@ func (s *RingpopSuite) TestDNSSRVMode() {
 		"duplicate entries should be removed",
 	)
 
-	//Expect unknown-duplicate.example.net to not resolve
+	// Expect unknown-duplicate.example.net to not resolve
 	_, err = cfg.DiscoveryProvider.Hosts()
 	s.NotNil(err)
 
-	//Remove known bad hosts from Unresolved list
+	// Remove known bad hosts from Unresolved list
 	provider.UnresolvedHosts = []string{
 		"service-a.example.net",
 		"service-b.example.net",
 		"badhostport",
 	}
 
-	//Expect badhostport to not seperate service name
+	// Expect badhostport to not seperate service name
 	_, err = cfg.DiscoveryProvider.Hosts()
 	s.NotNil(err)
 
-	//Remove known bad hosts from Unresolved list
+	// Remove known bad hosts from Unresolved list
 	provider.UnresolvedHosts = []string{
 		"service-a.example.net",
 		"service-b.example.net",

common/persistence/dataStoreInterfaces.go

@@ -33,12 +33,12 @@ import (
 )
 
 type (
-	//////////////////////////////////////////////////////////////////////
+	// ////////////////////////////////////////////////////////////////////
 	// Persistence interface is a lower layer of dataInterface.
 	// The intention is to let different persistence implementation(SQL,Cassandra/etc) share some common logic
 	// Right now the only common part is serialization/deserialization, and only ExecutionManager/HistoryManager need it.
 	// TaskManager are the same.
-	//////////////////////////////////////////////////////////////////////
+	// ////////////////////////////////////////////////////////////////////
 
 	// ShardStore is the lower level of ShardManager
 	ShardStore interface {
@@ -95,7 +95,7 @@ type (
 		Closeable
 		GetName() string
 		GetShardID() int
-		//The below three APIs are related to serialization/deserialization
+		// The below three APIs are related to serialization/deserialization
 		GetWorkflowExecution(ctx context.Context, request *InternalGetWorkflowExecutionRequest) (*InternalGetWorkflowExecutionResponse, error)
 		UpdateWorkflowExecution(ctx context.Context, request *InternalUpdateWorkflowExecutionRequest) error
 		ConflictResolveWorkflowExecution(ctx context.Context, request *InternalConflictResolveWorkflowExecutionRequest) error