Merge pull request #175 from Pix4D/re-enable-linter

Re-enable linter

Author: marco-m-pix4d

.github/workflows/ci.yml

@@ -6,29 +6,34 @@ name: ci
 
 env:
   go-version: 1.25.x
+  golangci-version: v2.8.0
+  gotestsum-version: v1.13.0
   task-version: v3.40.0
 
 jobs:
   all:
     runs-on: ubuntu-latest
     steps:
       - name: Install Go
-        uses: actions/setup-go@v5
+        uses: actions/setup-go@v6
         with:
           go-version: ${{ env.go-version }}
       - name: Install Task
         run: go install github.com/go-task/task/v3/cmd/task@${{ env.task-version }}
       - name: Checkout code
-        uses: actions/checkout@v4
+        uses: actions/checkout@v6
         with:
           # By default, actions/checkout will persist the GITHUB_TOKEN, so that further
           # steps in the job can perform authenticated git commands (that is: WRITE to
           # the repo). Following the Principle of least privilege, we disable this as long
           # as we don't need it.
           persist-credentials: false
-      - run: task ci:setup
-      # - run: task lint FIXME: re-enable
+      - name: golangci-lint
+        uses: golangci/golangci-lint-action@v9
+        with:
+          version: ${{ env.golangci-version }}
       - run: task build
+      - run: go install gotest.tools/gotestsum@${{ env.gotestsum-version }}
       - run: task test:all
         env:
           COGITO_TEST_OAUTH_TOKEN: ${{ secrets.COGITO_TEST_OAUTH_TOKEN }}

.golangci.yml

@@ -1,7 +1,10 @@
-# See https://golangci-lint.run/usage/configuration/#config-file
+# See https://golangci-lint.run/docs/configuration/file/
+
+version: "2"
 
 linters:
   # NOTE: `enable` _adds_ to the default linters!
   # To see the currently enabled linters, run `golangci-lint linters`
   enable:
     - gocritic
+    # - modernize LATER

Taskfile.yml

@@ -27,15 +27,15 @@ vars:
       "version": {"ref": "pizza"}
     }
   #
-  GOLANGCI_VERSION: v1.62.2
+  GOLANGCI_VERSION: v2.8.0
   GOTESTSUM_VERSION: v1.13.0
 
 tasks:
 
   install:deps:
     desc: Install tool dependencies.
     cmds:
-      # - go install github.com/golangci/golangci-lint/cmd/golangci-lint@{{.GOLANGCI_VERSION}}
+      - go install github.com/golangci/golangci-lint/v2/cmd/golangci-lint@{{.GOLANGCI_VERSION}}
       - go install gotest.tools/gotestsum@{{.GOTESTSUM_VERSION}}
 
   lint:
@@ -296,14 +296,6 @@ tasks:
     status:
       - test -z "{{.IS_RELEASE}}"
 
-  ci:setup:
-    desc: Useful only when running under CI.
-    cmds:
-      - task: install:deps
-      # Running "go mod download" is optional, since "go build" would do it anyway.
-      # We run it explicitly to make the output of "go build" more focused.
-      - cmd: go mod download -x
-
   # When using GitHub Actions, add this snippet at the end of the workflow:
   #    - run: docker logout
   #    # Always remove credentials, also if any previous step failed.

cogito/ghcommitsink_test.go

@@ -57,14 +57,18 @@ func TestSinkGitHubCommitStatusSendGhAppSuccess(t *testing.T) {
 			dec := json.NewDecoder(r.Body)
 			if err := dec.Decode(&ghReq); err != nil {
 				w.WriteHeader(http.StatusTeapot)
-				fmt.Fprintln(w, "test: decoding request:", err)
+				if _, err := fmt.Fprintln(w, "test: decoding request:", err); err != nil {
+					t.Fatalf("writing response: %s", err)
+				}
 				return
 			}
 		}
 
 		w.WriteHeader(http.StatusCreated)
 		if r.URL.String() == "/app/installations/12345/access_tokens" {
-			fmt.Fprintln(w, `{"token": "dummy_installation_token"}`)
+			if _, err := fmt.Fprintln(w, `{"token": "dummy_installation_token"}`); err != nil {
+				t.Fatalf("writing token: %s", err)
+			}
 			return
 		}
 	}

googlechat/googlechat.go

@@ -102,7 +102,11 @@ func TextMessage(
 	if err != nil {
 		return MessageReply{}, fmt.Errorf("TextMessage: send: %s", RedactErrorURL(err))
 	}
-	defer resp.Body.Close()
+	defer func() {
+		if err := resp.Body.Close(); err != nil {
+			log.Info("closing-response-body", "error", err)
+		}
+	}()
 	elapsed := time.Since(start)
 	log.Debug(
 		"http-request",

testhelp/testhelper.go

@@ -113,19 +113,23 @@ func CopyDir(dst string, src string, dirRenamer Renamer, templatedata TemplateDa
 	return nil
 }
 
-func copyFile(dstPath string, srcPath string, templatedata TemplateData) error {
+func copyFile(dstPath string, srcPath string, templatedata TemplateData) (allErrs error) {
 	srcFile, err := os.Open(srcPath)
 	if err != nil {
 		return fmt.Errorf("opening src file: %w", err)
 	}
-	defer srcFile.Close()
+	defer func() {
+		allErrs = errors.Join(allErrs, srcFile.Close())
+	}()
 
 	// We want an error if the file already exists
 	dstFile, err := os.OpenFile(dstPath, os.O_RDWR|os.O_CREATE|os.O_EXCL, 0o660)
 	if err != nil {
 		return fmt.Errorf("creating dst file: %w", err)
 	}
-	defer dstFile.Close()
+	defer func() {
+		allErrs = errors.Join(allErrs, dstFile.Close())
+	}()
 
 	if len(templatedata) == 0 {
 		_, err = io.Copy(dstFile, srcFile)

testhelp/testserver.go

@@ -32,7 +32,11 @@ func SpyHttpServer(request any, reply any, theUrl **url.URL, successCode int,
 			dec := json.NewDecoder(req.Body)
 			if err := dec.Decode(request); err != nil {
 				w.WriteHeader(http.StatusTeapot)
-				fmt.Fprintln(w, "test: decoding request:", err)
+				if _, err := fmt.Fprintln(w, "test: decoding request:", err); err != nil {
+					// In production one would normally log this kind of error.
+					// Here we are in a test, so it is appropriate to panic.
+					panic(fmt.Errorf("writing response: %s", err))
+				}
 				return
 			}