chore: Enable go fmt as a lint check for Go code (kubeflow#11830)

Signed-off-by: Caroline DeVoto <[email protected]>
Co-authored-by: Caroline DeVoto <[email protected]>

Author: cmdevoto

.github/workflows/pre-commit.yml

@@ -0,0 +1,21 @@
+name: pre-commit
+
+on:
+  push:
+    branches:
+      - master
+  pull_request:
+    branches:
+      - master
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v3
+    - uses: actions/setup-python@v3
+    - uses: pre-commit/[email protected]
+      # This is set to only run the golangci-lint pre-commit hooks 
+      # Remove in a later PR to run all hooks
+      with:
+        extra_args: golangci-lint --all-files

.golangci.yaml

@@ -1,3 +1,5 @@
+version: "2"
+
 run:
   timeout: 30m
   skip-files:
@@ -11,17 +13,20 @@ linters:
   disable-all: true
   enable: # please keep this alphabetized
     - gocritic
-    - gosimple
     - govet
     - ineffassign
     - misspell
     - staticcheck
-    - stylecheck
     - unused
+  settings:
+    misspell:
+      locale: US
+    staticcheck:
+      checks:
+        - "all"
 
-linters-settings: # please keep this alphabetized
-  misspell:
-    locale: US
-  staticcheck:
-    checks:
-      - "all"
+formatters:
+  disable-all: true
+  enable:
+    - gofmt
+    - goimports

.pre-commit-config.yaml

@@ -26,7 +26,7 @@ repos:
   # add comment "noqa" to ignore an import that should not be removed
   # (e.g., for an import with desired side-effects)
   - repo: https://github.com/hadialqattan/pycln
-    rev: v2.1.1
+    rev: v2.5.0
     hooks:
       - id: pycln
         name: pycln
@@ -43,7 +43,7 @@ repos:
     hooks:
       - id: yapf
   - repo: https://github.com/pycqa/docformatter
-    rev: v1.4
+    rev: v1.7.7
     hooks:
       - id: docformatter
         name: docformatter
@@ -55,7 +55,7 @@ repos:
 
   # Golang pre-submit hooks
   - repo: https://github.com/golangci/golangci-lint
-    rev: v1.64.8
+    rev: v2.1.2
     hooks:
       - id: golangci-lint
         name: golangci-lint
@@ -65,3 +65,11 @@ repos:
         language: golang
         require_serial: true
         pass_filenames: false
+      - id: golangci-lint
+        name: golangci-lint fmt
+        description: Formatter for Go.
+        entry: golangci-lint fmt
+        types: [go]
+        language: golang
+        require_serial: true
+        pass_filenames: false

backend/Makefile

@@ -108,3 +108,14 @@ kind-load-driver-debug:
 
 .PHONY: kind-build-and-load-driver-debug
 kind-build-and-load-driver-debug: image_driver_debug kind-load-driver-debug
+
+.PHONY: lint-and-format
+lint-and-format: lint format
+
+.PHONY: lint
+lint:
+	golangci-lint run --new-from-rev HEAD --fix
+
+.PHONY: format
+format:
+	golangci-lint fmt

backend/src/agent/persistence/client/token_refresher.go

@@ -1,10 +1,11 @@
 package client
 
 import (
-	log "github.com/sirupsen/logrus"
 	"os"
 	"sync"
 	"time"
+
+	log "github.com/sirupsen/logrus"
 )
 
 type TokenRefresherInterface interface {

backend/src/agent/persistence/worker/metrics_reporter_test.go

@@ -46,7 +46,7 @@ func TestReportMetrics_NoCompletedNode_NoOP(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeRunning,
@@ -72,7 +72,7 @@ func TestReportMetrics_NoRunID_NoOP(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:    "node-1",
 					Phase: workflowapi.NodeSucceeded,
 				},
@@ -99,7 +99,7 @@ func TestReportMetrics_NoArtifact_NoOP(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -127,7 +127,7 @@ func TestReportMetrics_NoMetricsArtifact_NoOP(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -156,7 +156,7 @@ func TestReportMetrics_Succeed(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -220,7 +220,7 @@ func TestReportMetrics_EmptyArchive_Fail(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -262,7 +262,7 @@ func TestReportMetrics_MultipleFilesInArchive_Fail(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "MY_NAME-template-1-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -306,7 +306,7 @@ func TestReportMetrics_InvalidMetricsJSON_Fail(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -349,15 +349,15 @@ func TestReportMetrics_InvalidMetricsJSON_PartialFail(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
 					Outputs: &workflowapi.Outputs{
 						Artifacts: []workflowapi.Artifact{{Name: "mlpipeline-metrics"}},
 					},
 				},
-				"node-2": workflowapi.NodeStatus{
+				"node-2": {
 					ID:           "node-2",
 					TemplateName: "template-2",
 					Phase:        workflowapi.NodeSucceeded,
@@ -400,12 +400,12 @@ func TestReportMetrics_InvalidMetricsJSON_PartialFail(t *testing.T) {
 	expectedMetricsRequest := &api.ReportRunMetricsRequest{
 		RunId: "run-1",
 		Metrics: []*api.RunMetric{
-			&api.RunMetric{
+			{
 				Name:   "accuracy",
 				NodeId: "MY_NAME-template-2-2",
 				Value:  &api.RunMetric_NumberValue{NumberValue: 0.77},
 			},
-			&api.RunMetric{
+			{
 				Name:   "logloss",
 				NodeId: "MY_NAME-template-2-2",
 				Value:  &api.RunMetric_NumberValue{NumberValue: 1.2},
@@ -432,7 +432,7 @@ func TestReportMetrics_CorruptedArchiveFile_Fail(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -473,7 +473,7 @@ func TestReportMetrics_MultiplMetricErrors_TransientErrowWin(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,
@@ -498,19 +498,19 @@ func TestReportMetrics_MultiplMetricErrors_TransientErrowWin(t *testing.T) {
 		})
 	pipelineFake.StubReportRunMetrics(&api.ReportRunMetricsResponse{
 		Results: []*api.ReportRunMetricsResponse_ReportRunMetricResult{
-			&api.ReportRunMetricsResponse_ReportRunMetricResult{
+			{
 				MetricNodeId: "node-1",
 				MetricName:   "accuracy",
 				Status:       api.ReportRunMetricsResponse_ReportRunMetricResult_OK,
 			},
 			// Invalid argument error triggers permanent error
-			&api.ReportRunMetricsResponse_ReportRunMetricResult{
+			{
 				MetricNodeId: "node-1",
 				MetricName:   "log loss",
 				Status:       api.ReportRunMetricsResponse_ReportRunMetricResult_INVALID_ARGUMENT,
 			},
 			// Internal error triggers transient error
-			&api.ReportRunMetricsResponse_ReportRunMetricResult{
+			{
 				MetricNodeId: "node-1",
 				MetricName:   "accuracy",
 				Status:       api.ReportRunMetricsResponse_ReportRunMetricResult_INTERNAL_ERROR,
@@ -537,7 +537,7 @@ func TestReportMetrics_Unauthorized(t *testing.T) {
 		},
 		Status: workflowapi.WorkflowStatus{
 			Nodes: map[string]workflowapi.NodeStatus{
-				"node-1": workflowapi.NodeStatus{
+				"node-1": {
 					ID:           "node-1",
 					TemplateName: "template-1",
 					Phase:        workflowapi.NodeSucceeded,

backend/src/apiserver/config/config.go

@@ -17,6 +17,9 @@ package config
 import (
 	"encoding/json"
 	"fmt"
+	"os"
+	"time"
+
 	"github.com/golang/glog"
 	"github.com/kubeflow/pipelines/backend/src/apiserver/client"
 	"github.com/kubeflow/pipelines/backend/src/apiserver/common"
@@ -25,8 +28,6 @@ import (
 	"github.com/kubeflow/pipelines/backend/src/apiserver/server"
 	"github.com/kubeflow/pipelines/backend/src/common/util"
 	"google.golang.org/grpc/codes"
-	"os"
-	"time"
 )
 
 // deprecated

backend/src/apiserver/config/config_test.go

@@ -16,6 +16,10 @@ package config
 
 import (
 	"encoding/json"
+	"os"
+	"path/filepath"
+	"testing"
+
 	"github.com/kubeflow/pipelines/backend/src/apiserver/list"
 	"github.com/kubeflow/pipelines/backend/src/apiserver/model"
 	"github.com/kubeflow/pipelines/backend/src/apiserver/resource"
@@ -24,9 +28,6 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc/codes"
-	"os"
-	"path/filepath"
-	"testing"
 )
 
 func fakeResourceManager() *resource.ResourceManager {

backend/src/apiserver/config/proxy/config.go

@@ -14,8 +14,9 @@
 package proxy
 
 import (
-	k8score "k8s.io/api/core/v1"
 	"os"
+
+	k8score "k8s.io/api/core/v1"
 )
 
 const (

backend/src/apiserver/config/proxy/config_test.go

@@ -15,11 +15,12 @@ package proxy
 
 import (
 	"fmt"
+	"os"
+	"testing"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	k8score "k8s.io/api/core/v1"
-	"os"
-	"testing"
 )
 
 func TestNewConfigFromEnvVars(t *testing.T) {