Skip to content

fix/ci: resolve all golangci-lint static check failures; drop revive#700

Merged
johnramsden merged 2 commits intocanonical:mainfrom
johnramsden:john/check-static
Apr 8, 2026
Merged

fix/ci: resolve all golangci-lint static check failures; drop revive#700
johnramsden merged 2 commits intocanonical:mainfrom
johnramsden:john/check-static

Conversation

@johnramsden
Copy link
Copy Markdown
Member

Description

Address every failure reported by golangci-lint under make check-static and add a static-checks CI job so regressions are caught automatically.

errcheck:

  • Propagate json.Unmarshal error in ceph/config.go
  • Capture and propagate snapRestart error in ceph/services.go
  • Check clientConfigUpdate return value in api/client_configs.go handlers
  • Check configChangeRefresh return value in api/configs.go handlers
  • Check SetAPIObjectID return value in api/ops_replication.go
  • Log errors from fire-and-forget goroutines in api/client_configs.go and api/configs.go (adds logger import to configs.go)
  • Suppress unchecked errors with _ = in test SetupTest helpers (tests/testutils.go, common/storage_test.go, ceph/services_test.go)
  • Use _ = for cobra MarkFlagRequired calls in pool.go, replication_demote.go, replication_disable_cephfs.go, replication_enable_cephfs.go, replication_enable_rbd.go, replication_promote.go

unused:

  • Delete unused functions: cephFSSnapshotMirrorDisableVolume, verifyMgrModuleEnabled, enableCephFSMgrModules, rgwCreateServiceDatabase
  • Remove unused struct fields: cmdLogSetLevel.logLevel, cmdReplicationList.{poolName,json}
  • Clean up now-orphaned imports in manager.go (strings, constants) and rgw.go (database/sql, database)

staticcheck:

  • Remove deprecated rand.Seed init() in cmd/microcephd/main.go; drop math/rand and time imports
  • Replace golang.org/x/crypto/ssh/terminal with golang.org/x/term in remote_list.go, replication_list_rbd.go, replication_status_rbd.go
  • Replace fmt.Errorf(errStr) with errors.New(errStr) in disk_add.go
  • Pass context.TODO() instead of nil to slog Logger.Enabled in clilogger/clilogger.go and logger/logger.go
  • Remove dead response append loop in ceph/cephfs_volume.go (SA4010)

gosimple:

  • Replace fmt.Sprintf("%s", volume) with string(volume) in cephfs_mirror.go

ineffassign:

  • Remove duplicate ServicePlacementHandler call in ceph/service_placement_nfs_test.go whose result was immediately overwritten before being checked

Remove revive from Makefile check-static target:

  • golangci-lint already covers the issues that matter (correctness, real bugs); revive adds only cosmetic style rules that overlap with golint and can be added back via .golangci.yml incrementally if desired

Add static-checks CI job (.github/workflows/tests.yml):

  • Mirrors the existing unit-tests job setup (ubuntu-24.04, dqlite PPA, Go 1.22) and runs make check-static on every push and pull request

Fixes #675

Type of change

Delete options that are not relevant.

  • Clean code (code refactor, test updates; does not introduce functional changes)

Contributor checklist

Please check that you have:

  • self-reviewed the code in this PR
  • added code comments, particularly in less straightforward areas
  • checked and added or updated relevant documentation
  • checked and added or updated relevant release notes
  • added tests to verify effectiveness of this change

@johnramsden johnramsden linked an issue Mar 26, 2026 that may be closed by this pull request
Address every failure reported by `golangci-lint` under `make check-static`
and add a static-checks CI job so regressions are caught automatically.

errcheck:
- Propagate json.Unmarshal error in ceph/config.go
- Capture and propagate snapRestart error in ceph/services.go
- Check clientConfigUpdate return value in api/client_configs.go handlers
- Check configChangeRefresh return value in api/configs.go handlers
- Check SetAPIObjectID return value in api/ops_replication.go
- Log errors from fire-and-forget goroutines in api/client_configs.go
  and api/configs.go (adds logger import to configs.go)
- Suppress unchecked errors with _ = in test SetupTest helpers
  (tests/testutils.go, common/storage_test.go, ceph/services_test.go)
- Use _ = for cobra MarkFlagRequired calls in pool.go,
  replication_demote.go, replication_disable_cephfs.go,
  replication_enable_cephfs.go, replication_enable_rbd.go,
  replication_promote.go

unused:
- Delete unused functions: cephFSSnapshotMirrorDisableVolume,
  verifyMgrModuleEnabled, enableCephFSMgrModules, rgwCreateServiceDatabase
- Remove unused struct fields: cmdLogSetLevel.logLevel,
  cmdReplicationList.{poolName,json}
- Clean up now-orphaned imports in manager.go (strings, constants)
  and rgw.go (database/sql, database)

staticcheck:
- Remove deprecated rand.Seed init() in cmd/microcephd/main.go;
  drop math/rand and time imports
- Replace golang.org/x/crypto/ssh/terminal with golang.org/x/term
  in remote_list.go, replication_list_rbd.go, replication_status_rbd.go
- Replace fmt.Errorf(errStr) with errors.New(errStr) in disk_add.go
- Pass context.TODO() instead of nil to slog Logger.Enabled in
  clilogger/clilogger.go and logger/logger.go
- Remove dead response append loop in ceph/cephfs_volume.go (SA4010)

gosimple:
- Replace fmt.Sprintf("%s", volume) with string(volume) in cephfs_mirror.go

ineffassign:
- Remove duplicate ServicePlacementHandler call in
  ceph/service_placement_nfs_test.go whose result was immediately
  overwritten before being checked

Remove revive from Makefile check-static target:
- golangci-lint already covers the issues that matter (correctness,
  real bugs); revive adds only cosmetic style rules that overlap with
  golint and can be added back via .golangci.yml incrementally if desired

Add static-checks CI job (.github/workflows/tests.yml):
- Mirrors the existing unit-tests job setup (ubuntu-24.04, dqlite PPA,
  Go 1.22) and runs `make check-static` on every push and pull request

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: John Ramsden <john.ramsden@canonical.com>
@sabaini
Copy link
Copy Markdown
Collaborator

sabaini commented Apr 8, 2026

Hey @johnramsden great cleanup, thanks! One mechanical comment per convention we avoid the one line assing/test for better maintainability and readability

@UtkarshBhatthere UtkarshBhatthere added this to the Tentacle readiness milestone Apr 8, 2026
Per CONTRIBUTING.md, avoid `if err := f(); err != nil` — use separate
assign and test lines for maintainability.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: John Ramsden <john.ramsden@canonical.com>
@johnramsden johnramsden merged commit f3ce037 into canonical:main Apr 8, 2026
24 checks passed
@johnramsden johnramsden deleted the john/check-static branch April 8, 2026 23:59
skoech pushed a commit that referenced this pull request Apr 9, 2026
…700)

# Description

Address every failure reported by `golangci-lint` under `make
check-static` and add a static-checks CI job so regressions are caught
automatically.

errcheck:
- Propagate json.Unmarshal error in ceph/config.go
- Capture and propagate snapRestart error in ceph/services.go
- Check clientConfigUpdate return value in api/client_configs.go
handlers
- Check configChangeRefresh return value in api/configs.go handlers
- Check SetAPIObjectID return value in api/ops_replication.go
- Log errors from fire-and-forget goroutines in api/client_configs.go
and api/configs.go (adds logger import to configs.go)
- Suppress unchecked errors with _ = in test SetupTest helpers
(tests/testutils.go, common/storage_test.go, ceph/services_test.go)
- Use _ = for cobra MarkFlagRequired calls in pool.go,
replication_demote.go, replication_disable_cephfs.go,
replication_enable_cephfs.go, replication_enable_rbd.go,
replication_promote.go

unused:
- Delete unused functions: cephFSSnapshotMirrorDisableVolume,
verifyMgrModuleEnabled, enableCephFSMgrModules, rgwCreateServiceDatabase
- Remove unused struct fields: cmdLogSetLevel.logLevel,
cmdReplicationList.{poolName,json}
- Clean up now-orphaned imports in manager.go (strings, constants) and
rgw.go (database/sql, database)

staticcheck:
- Remove deprecated rand.Seed init() in cmd/microcephd/main.go; drop
math/rand and time imports
- Replace golang.org/x/crypto/ssh/terminal with golang.org/x/term in
remote_list.go, replication_list_rbd.go, replication_status_rbd.go
- Replace fmt.Errorf(errStr) with errors.New(errStr) in disk_add.go
- Pass context.TODO() instead of nil to slog Logger.Enabled in
clilogger/clilogger.go and logger/logger.go
- Remove dead response append loop in ceph/cephfs_volume.go (SA4010)

gosimple:
- Replace fmt.Sprintf("%s", volume) with string(volume) in
cephfs_mirror.go

ineffassign:
- Remove duplicate ServicePlacementHandler call in
ceph/service_placement_nfs_test.go whose result was immediately
overwritten before being checked

Remove revive from Makefile check-static target:
- golangci-lint already covers the issues that matter (correctness, real
bugs); revive adds only cosmetic style rules that overlap with golint
and can be added back via .golangci.yml incrementally if desired

Add static-checks CI job (.github/workflows/tests.yml):
- Mirrors the existing unit-tests job setup (ubuntu-24.04, dqlite PPA,
Go 1.22) and runs `make check-static` on every push and pull request

Fixes #675 

## Type of change

Delete options that are not relevant.

- Clean code (code refactor, test updates; does not introduce functional
changes)

## Contributor checklist

Please check that you have:

- [x] self-reviewed the code in this PR
- [x] added code comments, particularly in less straightforward areas
- [x] checked and added or updated relevant documentation
- [x] checked and added or updated relevant release notes
- [x] added tests to verify effectiveness of this change

---------

Signed-off-by: John Ramsden <john.ramsden@canonical.com>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

MicroCeph Quality Improvements check-static fails and is not in CI

3 participants