Skip to content

Upgrade to Go 1.24 and golangci-lint 2.11.4#140

Open
dangoor wants to merge 2 commits intomainfrom
dangoor/go-and-linter-upgrade
Open

Upgrade to Go 1.24 and golangci-lint 2.11.4#140
dangoor wants to merge 2 commits intomainfrom
dangoor/go-and-linter-upgrade

Conversation

@dangoor
Copy link
Copy Markdown
Contributor

@dangoor dangoor commented Apr 9, 2026

This updates the Go version we're using and brings us up to speed
with the latest golangci-lint. This fixes the lint issues that
inevitably came up with the transition.

This updates the Go version we're using and brings us up to speed
with the latest golangci-lint. This fixes the lint issues that
inevitably came up with the transition.
@dangoor dangoor requested a review from elrayle as a code owner April 9, 2026 20:02
Copilot AI review requested due to automatic review settings April 9, 2026 20:02
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates the repository’s Go toolchain and golangci-lint configuration to newer versions, along with associated lint-driven code/comment adjustments.

Changes:

  • Bump GitHub Actions workflows to Go 1.24.7 and golangci-lint to v2.11.4.
  • Update .golangci.yaml for golangci-lint v2 config format (including formatter configuration).
  • Address new lint findings (errcheck/gosec) and adjust Go package/documentation comments.
Show a summary per file
File Description
spdxexp/spdxlicenses/license_ranges.go Updates exported function comment for LicenseRanges.
spdxexp/spdxlicenses/doc.go Reflows package documentation comment to match Go doc conventions.
spdxexp/doc.go Reflows package documentation comment to match Go doc conventions.
spdxexp/benchmark_setup_test.go Adjusts benchmark output writes to satisfy errcheck.
cmd/spdx-validate/main.go Adds package comment, gosec suppression for CLI file open, and errcheck-related write changes.
cmd/spdx-validate/main_test.go Adds gosec suppression for temp-file open and adjusts deferred close.
.golangci.yaml Migrates to golangci-lint v2 config layout; enables gofmt/goimports as formatters.
.github/workflows/test.yaml Updates CI Go version to 1.24.7.
.github/workflows/lint.yaml Updates CI Go version to 1.24.7 and golangci-lint to v2.11.4.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (2)

cmd/spdx-validate/main.go:50

  • os.Exit(1) inside RunE will bypass deferred cleanup (including the deferred file Close() above) and makes this command harder to test/compose. Prefer returning an error (or a typed/sentinel error handled in main) and let main decide the exit code so defers still run.
		ok, err := validateExpressions(r, os.Stderr)
		if err != nil {
			return err
		}
		if !ok {
			os.Exit(1)
		}
		return nil

cmd/spdx-validate/main.go:92

  • The summary fmt.Fprintf write error is ignored, which can lead to returning (false, nil) even when output could not be written. Consider returning an error on write failure to keep the function’s result consistent with what was actually reported.
	if failures > 0 {
		_, _ = fmt.Fprintf(w, "%d of %d expressions failed validation\n", failures, lineNum)
		return false, nil
  • Files reviewed: 9/9 changed files
  • Comments generated: 1

Comment on lines 75 to 79
valid, _ := spdxexp.ValidateLicenses([]string{line})
if !valid {
failures++
fmt.Fprintf(w, "line %d: invalid SPDX expression: %q\n", lineNum, line)
_, _ = fmt.Fprintf(w, "line %d: invalid SPDX expression: %q\n", lineNum, line)
}
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validateExpressions currently discards the error from fmt.Fprintf when writing per-line failures. Since w is caller-provided, write errors should be propagated (e.g., return (false, err)) so callers can detect/report output failures.

This issue also appears on line 90 of the same file.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really care about this at this time. Was basically just keeping the behavior as-is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants