chore(lint): bump golangci lint config to v2, address lint findings by mattsre · Pull Request #365 · streamnative/pulsar-resources-operator

mattsre · 2025-12-28T01:58:26Z

Bumps the golangci-lint config to v2, address some lint findings from new config. Config was migrated with automation - https://golangci-lint.run/docs/product/migration-guide/

Motivation

New versions of golangci-lint don't support the old v1 config file format. Maintaining two versions of golangci-lint locally can be annoying, especially when you are integrating golangci-lint into your editor for lsp 😄

Modifications

Add some local dev tools to gitignore
migrate the golangci-lint config file to v2 format

Verifying this change

Make sure that the change passes the CI checks.

This change is a trivial rework / code cleanup without any test coverage.

Documentation

no-need-doc

Only upgrading tooling separate from the operator itself

maxsxu · 2026-01-05T03:16:08Z

pkg/admin/impl.go

-	if topicMeta.Partitions > 0 {
-		nonPartitioned = false
-	}
+	nonPartitioned := topicMeta.Partitions < 1


@mattsre Nice catch! 👍

Copilot

Pull request overview

This PR upgrades the golangci-lint tooling from v1.x to v2.x by migrating the configuration file format and updating version references across the codebase. The upgrade also addresses several lint findings that were identified by the new linter version.

Key Changes:

Migrated .golangci.yml from v1 to v2 format, reorganizing linters, settings, and exclusions
Updated golangci-lint from v1.55.2/v1.64 to v2.7.2 across scripts and CI workflows
Addressed lint findings by simplifying redundant code patterns (boolean logic, ObjectMeta field access)

Reviewed changes

Copilot reviewed 6 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`.golangci.yml`	Migrated configuration to v2 format with restructured linters, settings, and exclusions sections
`scripts/lint.sh`	Updated golangci-lint installation version from v1.55.2 to v2.7.2
`.github/workflows/golangci-lint.yml`	Upgraded GitHub action from v6 to v8 and linter version to v2.7.2
`pkg/admin/impl.go`	Simplified boolean assignment logic per lint suggestion
`controllers/serviceaccountbinding_controller.go`	Simplified ObjectMeta.Name access to direct Name field access
`controllers/secret_controller.go`	Simplified ObjectMeta.DeletionTimestamp access to direct DeletionTimestamp field access
`.gitignore`	Added local development tool files to ignore list
`go.sum`	Added missing go.mod entry for dependency

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

Copilot · 2026-01-05T03:39:56Z

.golangci.yml

+      - linters:
+          - revive
+        text: 'exported: exported method .*\.(Reconcile|SetupWithManager|SetupWebhookWithManager) should have comment or be unexported'
+      - linters:
+          - errcheck
+        text: Error return value of .((os\.)?std(out|err)\..*|.*Close|.*Flush|os\.Remove(All)?|.*print(f|ln)?|os\.(Un)?Setenv). is not checked
+      - linters:
+          - revive
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+        source: (func|type).*Fake.*
+      - linters:
+          - revive
+        path: fake_\.go
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - revive
+        path: cmd/clusterctl/internal/test/providers.*.go
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - revive
+        path: (framework|e2e)/.*.go
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - unparam
+        text: always receives
+      - path: _test\.go
+        text: should not use dot imports
+      - path: (framework|e2e)/.*.go
+        text: should not use dot imports
+      - path: _test\.go
+        text: cyclomatic complexity
+      - linters:
+          - gocritic
+        text: 'appendAssign: append result not assigned to the same slice'
+      - linters:
+          - ifshort
+        path: controllers/mdutil/util.go
+        text: variable .* is only used in the if-statement
+      - linters:
+          - staticcheck
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'SA1019: in.(.+) is deprecated'
+      - linters:
+          - revive
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: exported (method|function|type|const) (.+) should have comment or be unexported
+      - linters:
+          - revive
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'var-naming: don''t use underscores in Go names;'
+      - linters:
+          - revive
+        path: .*(api|types)\/.*\/conversion.*\.go$
+        text: 'receiver-naming: receiver name'


Multiple exclusion rules reference the revive linter (lines 99, 105, 109, 113, 117, 141, 145, 149), but revive is not in the enabled linters list. These exclusions will have no effect. Either add revive to the enabled linters if these exclusions are needed, or remove these exclusion rules to clean up the configuration.

Enabled revive

@maxsxu or @freeznet Enabling the revive linter adds a lot of the following lint failures:

exported method|type|const ... should have comment or be unexported

and many like

var-naming: struct field SqlScript should be SQLScript

Most of the revive lints seem stylistic, I don't really like them but wanted to ask your thoughts? Do you want it enabled or should I leave it disabled and remove the config associated with it?

.golangci.yml

Copilot · 2026-01-05T03:39:57Z

.golangci.yml

+    godot:
+      scope: toplevel
+      exclude:
+        - ^ \+.*
+        - ^ ANCHOR.*


The godot linter has settings configured but is not enabled in the linters list (line 26). These settings will have no effect. If godot linting is desired, add it to the enabled linters list. Otherwise, consider removing these unused settings to keep the configuration clean.

Suggested change

godot:

scope: toplevel

exclude:

- ^ \+.*

- ^ ANCHOR.*

Removing godot linter config - godot is used for adding "." at the end of sentenences however most of the comments/docs in the repo do not follow this pattern so it doesn't seem like this should be enabled

mattsre · 2026-01-27T18:03:12Z

@freeznet Is there anything you need from me on this PR? I see the CI is failing with some tmate session errors

maxsxu · 2026-03-24T13:38:11Z

I've reviewed the changes and the functional Go code looks fine. However, there is a regression introduced by the golangci-lint v2 migration:

Moving gofmt and goimports into the new formatters section in .golangci.yml (without changing the invocation path) means these checks will stop running in both CI (.github/workflows/golangci-lint.yml) and scripts/lint.sh. Since they are no longer considered linters in v2, running golangci-lint run will no longer fail on misformatted files or import ordering regressions, allowing formatting issues to merge unnoticed.

maxsxu

PR Code Review (Dual-Engine: Codex + Gemini Fallback)

Note: The Gemini engine encountered an initialization error (missing API key), so this review relies on Codex's deep analysis combined with a manual fallback review.

Summary of Changes

Upgrades golangci-lint to v2.7.2, along with the GitHub Action to v8.
Restructures .golangci.yml to the v2 format, modifying linter exclusions and moving formatters.
Adds .jj/, docker-compose.yaml, and mise.toml to .gitignore.
Simplifies conditional logic in pkg/admin/impl.go.
Handles the promotion of DeletionTimestamp in secret_controller.go.

⚠️ Critical Findings

Formatting Checks Bypassed (P1)
Moving gofmt and goimports under formatters: in .golangci.yml means they are no longer evaluated by golangci-lint run. Since the GitHub Action and scripts/lint.sh both still invoke run by default, this will cause CI to silently ignore formatting and import-order regressions.
Suggestion: Either keep them under linters or ensure CI and scripts explicitly run the formatting checks.
Local Dev & CI Caching Issue for golangci-lint Binary (P2)
In scripts/lint.sh, the script only downloads the new v2.7.2 binary if ${POP_HOME}/bin/golangci-lint does not exist. Developers and CI runners with a cached v1 binary will skip the download, run the old v1 binary against the new version: "2" config, and fail parsing.
Suggestion: Update the script to check the version as well. For example:
```
if [ ! -f "${POP_HOME}/bin/golangci-lint" ] || ! "${POP_HOME}/bin/golangci-lint" version | grep -q "2.7.2"; then
    wget -O - -q https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s v2.7.2
fi
```

👍 Positives

The refactoring in pkg/admin/impl.go (nonPartitioned := topicMeta.Partitions < 1) is concise and clean.
Controller updates properly handle the upstream Kubernetes API DeletionTimestamp changes.

Recommendation

Changes Requested: Please address the scripts/lint.sh caching issue and verify how formatting checks are enforced in CI before merging.

mattsre requested review from a team as code owners December 28, 2025 01:58

github-actions bot assigned mattsre Dec 28, 2025

github-actions bot added the no-need-doc This pr does not need any document label Dec 28, 2025

mattsre added 3 commits December 27, 2025 21:24

chore(lint): bump golangci lint config to v2, address lint findings

1596e01

chore(lint): address QF1007 staticcheck rule

d9d9379

chore(lint): address QF1008 staticcheck rule

608024c

mattsre force-pushed the golang-lintfix branch from 3bcfe35 to 608024c Compare December 28, 2025 02:27

chore(lint): bump golangci-lint version in CI

cc71d16

mattsre mentioned this pull request Dec 28, 2025

refactor: use OSS Pulsar image and charts for e2e tests #366

Open

2 tasks

maxsxu reviewed Jan 5, 2026

View reviewed changes

freeznet requested a review from Copilot January 5, 2026 03:34

Copilot started reviewing on behalf of freeznet January 5, 2026 03:34 View session

Copilot AI reviewed Jan 5, 2026

View reviewed changes

mattsre and others added 3 commits January 6, 2026 21:39

chore: cleanup golangci lints

52372c3

Merge branch 'main' into golang-lintfix

4284480

Merge branch 'main' into golang-lintfix

852b4d5

Merge branch 'main' into golang-lintfix

ae2deb4

maxsxu reviewed Mar 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(lint): bump golangci lint config to v2, address lint findings#365

chore(lint): bump golangci lint config to v2, address lint findings#365
mattsre wants to merge 8 commits intostreamnative:mainfrom
mattsre:golang-lintfix

mattsre commented Dec 28, 2025 •

edited

Loading

Uh oh!

maxsxu Jan 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

mattsre Jan 7, 2026

Uh oh!

mattsre Jan 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 5, 2026

Uh oh!

mattsre Jan 7, 2026

Uh oh!

mattsre commented Jan 27, 2026

Uh oh!

maxsxu commented Mar 24, 2026

Uh oh!

maxsxu left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

mattsre commented Dec 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Verifying this change

Documentation

Uh oh!

maxsxu Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

mattsre Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

mattsre Jan 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

Uh oh!

mattsre Jan 7, 2026

Choose a reason for hiding this comment

Uh oh!

mattsre commented Jan 27, 2026

Uh oh!

maxsxu commented Mar 24, 2026

Uh oh!

maxsxu left a comment

Choose a reason for hiding this comment

PR Code Review (Dual-Engine: Codex + Gemini Fallback)

Summary of Changes

⚠️ Critical Findings

👍 Positives

Recommendation

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mattsre commented Dec 28, 2025 •

edited

Loading

mattsre Jan 7, 2026 •

edited

Loading