Updated formatting (#82)

* run goimports

* run golangci-lint

* add lint test

* run imports prior to building

* install goimports inside container

* run goimports

* cleanup

.github/workflows/lint.yaml

@@ -0,0 +1,37 @@
+name: Lint
+
+env:
+  GO_VERSION: '1.20.4'
+  LINTER_VERSION: 'v1.52.2'
+
+on: push
+
+jobs:
+  lint:
+    runs-on: ubuntu-20.04
+    steps:
+      - uses: actions/setup-go@v3
+        with:
+          go-version: ${{ env.GO_VERSION }}
+      - uses: actions/checkout@v2
+      - name: Cache Vendor
+        uses: actions/cache@v2
+        env:
+          cache-name: cache-vendor-v1
+        with:
+          path: vendor
+          key: ${{ env.cache-name }}-${{ env.GO_VERSION }}-${{ hashFiles('go.mod') }}-${{ hashFiles('go.sum') }}
+          restore-keys: |
+            ${{ env.cache-name }}-${{ env.GO_VERSION }}-
+            ${{ env.cache-name }}-
+
+      - name: Populate Vendor
+        shell: bash
+        run: |
+          make vendor
+
+      - shell: bash
+        run: go install github.com/golangci/golangci-lint/cmd/golangci-lint@${{ env.LINTER_VERSION }}
+
+      - shell: bash
+        run: golangci-lint run --out-format=github-actions

Dockerfile

@@ -8,8 +8,9 @@ FROM golang:1.20.4-buster as intermediate
 # Make a directory to place pprof files in. Typically used for itests.
 RUN mkdir /perf
 
-# Build dependencies
+# Build/test dependencies
 RUN go install github.com/golang/mock/mockgen@v1.6.0
+RUN go install golang.org/x/tools/cmd/goimports@latest
 
 WORKDIR /go/src/github.com/Nextdoor/pg-bifrost.git/
 

Makefile

@@ -10,6 +10,18 @@ ifeq ($(CI),true)
 	GO_LDFLAGS += -X main.GitRevision=$(GIT_REVISION) -X main.Version=$(GIT_TAG_VERSION)
 endif
 
+vendor: go.sum go.mod
+	go mod vendor -v
+
+check_imports:
+	@echo "Checking goimports for imports and formatting..."
+	goimports -l -d .
+	@goimports -l -d . | xargs echo | xargs test -z 2> /dev/null
+
+lint: vet check_imports
+	@echo "Running golangci-lint"
+	golangci-lint run
+
 vet:
 	@echo "Running go vet ..."
 	go list ./...  | xargs go vet
@@ -20,7 +32,7 @@ generate:
 		go generate ./... ;\
 	fi
 
-test: generate vet
+test: generate check_imports
 	go clean -testcache || true
 	@echo "Executing tests ..."
 	go test -race -v ${GO_TEST_EXTRAS} ./...
@@ -62,4 +74,4 @@ docker_get_binary:
 	@$(DOCKER) cp "pg-bifrost-build":/pg-bifrost target/
 	@$(DOCKER) rm "pg-bifrost-build"
 
-.PHONY: clean test itests docker_build docker_get_binary
+.PHONY: clean test itests docker_build docker_get_binary vendor lint check_imports

app/config/headers.go

@@ -12,7 +12,7 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
- */
+*/
 
 package config
 

app/runner.go

@@ -17,11 +17,14 @@
 package app
 
 import (
+	"time"
+
 	"github.com/Nextdoor/pg-bifrost.git/app/config"
 	"github.com/Nextdoor/pg-bifrost.git/partitioner"
 	"github.com/Nextdoor/pg-bifrost.git/transport/batcher"
 	"github.com/jackc/pgx/v5/pgconn"
-	"time"
+
+	"os"
 
 	"github.com/Nextdoor/pg-bifrost.git/filter"
 	"github.com/Nextdoor/pg-bifrost.git/marshaller"
@@ -37,7 +40,6 @@ import (
 	"github.com/Nextdoor/pg-bifrost.git/transport/progress"
 	"github.com/cevaris/ordered_map"
 	"github.com/sirupsen/logrus"
-	"os"
 )
 
 var (
@@ -234,7 +236,7 @@ func (m Runner) shutdown() {
 
 	defer func() {
 		// recover if channel is already closed
-		recover()
+		_ = recover()
 	}()
 
 	log.Debug("closing global channels")

filter/filter.go

@@ -12,7 +12,7 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
- */
+*/
 
 package filter
 
@@ -99,7 +99,7 @@ func (f *Filter) shutdown() {
 
 	defer func() {
 		// recover if channel is already closed
-		recover()
+		_ = recover()
 	}()
 
 	log.Debug("closing output channel")

filter/filter_test.go

@@ -12,19 +12,20 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
- */
+*/
 
 package filter
 
 import (
 	"github.com/Nextdoor/pg-bifrost.git/replication"
 
+	"testing"
+	"time"
+
+	"github.com/Nextdoor/parselogical"
 	"github.com/Nextdoor/pg-bifrost.git/shutdown"
 	"github.com/Nextdoor/pg-bifrost.git/stats"
-	"github.com/Nextdoor/parselogical"
 	"github.com/stretchr/testify/assert"
-	"testing"
-	"time"
 )
 
 func _testMessage(relation string, operation string) *replication.WalMessage {

main/main.go

@@ -274,7 +274,7 @@ func runReplicate(
 			// If it's a SIGUSR1 and the cpuprofile flag was set, then dump pprof files and continue running.
 			// https://golang.org/pkg/runtime/pprof
 			if sig == syscall.SIGUSR1 && cpuprofile != "" {
-				if stoppedCpuProfiling != true {
+				if !stoppedCpuProfiling {
 					log.Warn("Stopping CPU profiling")
 					pprof.StopCPUProfile()
 					log.Infof("Wrote pprof cpu profile to: '%s'", cpuprofile)
@@ -292,15 +292,15 @@ func runReplicate(
 			// https://golang.org/pkg/runtime/pprof
 			if sig == syscall.SIGUSR2 && memprofile != "" {
 				time.Sleep(1 * time.Second)
-				memProfile(fmt.Sprintf(memprofile))
+				memProfile(memprofile)
 
 				continue
 			}
 
 			log.Info("pg-bifrost received a shutdown")
 
 			// Stop profiling on shutdown
-			if cpuprofile != "" && stoppedCpuProfiling != true {
+			if cpuprofile != "" && !stoppedCpuProfiling {
 				log.Warn("Stopping CPU profiling")
 				pprof.StopCPUProfile()
 			}

marshaller/marshalled_message.go

@@ -12,7 +12,7 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
- */
+*/
 
 package marshaller
 

marshaller/marshalled_message_test.go

@@ -12,7 +12,7 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
- */
+*/
 
 package marshaller
 

marshaller/marshaller.go

@@ -17,7 +17,7 @@
 package marshaller
 
 import (
-	"github.com/json-iterator/go"
+	jsoniter "github.com/json-iterator/go"
 
 	"os"
 	"time"
@@ -83,7 +83,7 @@ func (m Marshaller) shutdown() {
 
 	defer func() {
 		// recover if channel is already closed
-		recover()
+		_ = recover()
 	}()
 
 	log.Debug("closing output channel")

marshaller/marshaller_test.go

@@ -17,14 +17,15 @@
 package marshaller
 
 import (
+	"testing"
+	"time"
+
 	"github.com/Nextdoor/parselogical"
 	"github.com/Nextdoor/pg-bifrost.git/replication"
 	"github.com/Nextdoor/pg-bifrost.git/shutdown"
 	"github.com/Nextdoor/pg-bifrost.git/stats"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
-	"testing"
-	"time"
 )
 
 func init() {

partitioner/partitioner.go

@@ -12,18 +12,19 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
- */
+*/
 
 package partitioner
 
 import (
+	"os"
+	"strconv"
+
 	"github.com/Nextdoor/pg-bifrost.git/replication"
 	"github.com/Nextdoor/pg-bifrost.git/shutdown"
 	"github.com/Nextdoor/pg-bifrost.git/stats"
 	"github.com/Nextdoor/pg-bifrost.git/utils"
 	"github.com/sirupsen/logrus"
-	"os"
-	"strconv"
 )
 
 type PartitionMethod int
@@ -66,7 +67,7 @@ type Partitioner struct {
 
 	statsChan chan stats.Stat
 
-	method PartitionMethod
+	method  PartitionMethod
 	buckets int
 }
 
@@ -98,7 +99,7 @@ func (f *Partitioner) shutdown() {
 
 	defer func() {
 		// recover if channel is already closed
-		recover()
+		_ = recover()
 	}()
 
 	log.Debug("closing output channel")
@@ -144,16 +145,12 @@ func (f *Partitioner) Start() {
 		switch f.method {
 		case PART_METHOD_NONE:
 			partitionKey = ""
-			break
 		case PART_METHOD_TABLENAME:
 			partitionKey = msg.Pr.Relation
-			break
 		case PART_METHOD_TXN:
 			partitionKey = msg.Pr.Transaction
-			break
 		case PART_METHOD_TXN_BUCKET:
 			partitionKey = strconv.Itoa(utils.QuickHash(msg.Pr.Transaction, f.buckets))
-			break
 		}
 
 		msg.PartitionKey = partitionKey

partitioner/partitioner_test.go

@@ -12,18 +12,18 @@
   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
   See the License for the specific language governing permissions and
   limitations under the License.
- */
+*/
 
 package partitioner
 
 import (
+	"testing"
+
+	"github.com/Nextdoor/parselogical"
 	"github.com/Nextdoor/pg-bifrost.git/replication"
 	"github.com/Nextdoor/pg-bifrost.git/shutdown"
 	"github.com/Nextdoor/pg-bifrost.git/stats"
-	"github.com/Nextdoor/parselogical"
 	"github.com/stretchr/testify/assert"
-	"testing"
-	"time"
 )
 
 func _setupPartitioner(partMethod PartitionMethod, buckets int) (Partitioner, chan *replication.WalMessage, chan stats.Stat) {
@@ -36,24 +36,6 @@ func _setupPartitioner(partMethod PartitionMethod, buckets int) (Partitioner, ch
 	return p, in, statsChan
 }
 
-func _collectOutput(outputChan chan *replication.WalMessage) []*replication.WalMessage {
-	actual := make([]*replication.WalMessage, 0)
-
-	func() {
-		for {
-			select {
-			case <-time.After(5 * time.Millisecond):
-				// Got all the stats
-				return
-			case m := <-outputChan:
-				actual = append(actual, m)
-			}
-		}
-	}()
-
-	return actual
-}
-
 func _testMessage(relation string, transaction string) *replication.WalMessage {
 	pr := parselogical.ParseResult{
 		State:       parselogical.ParseState{},
@@ -84,7 +66,7 @@ func TestNone(t *testing.T) {
 	go p.Start()
 
 	in <- msg
-	outMsg := <- p.OutputChan
+	outMsg := <-p.OutputChan
 
 	assert.Equal(t, "", outMsg.PartitionKey)
 }
@@ -97,7 +79,7 @@ func TestTableName(t *testing.T) {
 	go p.Start()
 
 	in <- msg
-	outMsg := <- p.OutputChan
+	outMsg := <-p.OutputChan
 
 	assert.Equal(t, "users", outMsg.PartitionKey)
 }
@@ -110,7 +92,7 @@ func TestTransaction(t *testing.T) {
 	go p.Start()
 
 	in <- msg
-	outMsg := <- p.OutputChan
+	outMsg := <-p.OutputChan
 
 	assert.Equal(t, "19", outMsg.PartitionKey)
 }
@@ -121,10 +103,10 @@ func TestTransactionBuckets(t *testing.T) {
 	go p.Start()
 
 	in <- _testMessage("users", "111111")
-	outMsgA := <- p.OutputChan
+	outMsgA := <-p.OutputChan
 	assert.Equal(t, "4", outMsgA.PartitionKey)
 
 	in <- _testMessage("users", "333333")
-	outMsgB := <- p.OutputChan
+	outMsgB := <-p.OutputChan
 	assert.Equal(t, "1", outMsgB.PartitionKey)
-}
+}