chore(staticcheck): clean staticcheck warnings in libvirt, protobuf, and ledger#2
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Staticcheck-focused cleanup across Postgres execution ledger, libvirt adapter initialization, and generated protobuf code.
Changes:
- Resolve staticcheck S1021 in execution ledger by merging declaration/assignment.
- Replace deprecated
go-libvirtconstructor usage withNewWithDialerusing a unix dialer. - Suppress SA1019 warnings in generated protobuf output with a file-level ignore directive.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/repositories/postgres/execution_ledger.go | Simplifies QueryRow assignment to satisfy S1021. |
| internal/repositories/libvirt/adapter.go | Introduces a unix dialer and switches libvirt client construction to NewWithDialer. |
| internal/autoscaler/protos/externalgrpc.pb.go | Adds a file-level staticcheck ignore for SA1019 in generated protobuf code. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| //lint:file-ignore SA1019 Generated protobuf compatibility shims may reference deprecated APIs. | ||
|
|
There was a problem hiding this comment.
This file is marked as generated (protoc-gen-go). Adding a manual //lint:file-ignore directive here will be lost the next time the protobufs are regenerated. If protobuf regeneration is part of the normal workflow, prefer adding the suppression via the generator/config (e.g., golangci-lint config for generated files, or a protoc/gen option that emits the directive) so it persists across regenerations.
| //lint:file-ignore SA1019 Generated protobuf compatibility shims may reference deprecated APIs. |
| uri string | ||
| timeout time.Duration | ||
| } | ||
|
|
||
| var _ libvirtsocket.Dialer = (*libvirtUnixDialer)(nil) | ||
|
|
||
| func (d *libvirtUnixDialer) Dial() (net.Conn, error) { | ||
| return net.DialTimeout("unix", d.uri, d.timeout) |
There was a problem hiding this comment.
libvirtUnixDialer stores the unix socket path in a field named uri, but in this package uri is also used for libvirt connection URIs like qemu:///system (see tests/usages). This naming makes it easy to accidentally pass a non-socket URI and have Dial() try to net.DialTimeout("unix", "qemu:///system", ...). Consider renaming the field (and constructor arg) to socketPath (or similar) to make the expected format explicit.
| uri string | |
| timeout time.Duration | |
| } | |
| var _ libvirtsocket.Dialer = (*libvirtUnixDialer)(nil) | |
| func (d *libvirtUnixDialer) Dial() (net.Conn, error) { | |
| return net.DialTimeout("unix", d.uri, d.timeout) | |
| socketPath string | |
| timeout time.Duration | |
| } | |
| var _ libvirtsocket.Dialer = (*libvirtUnixDialer)(nil) | |
| func (d *libvirtUnixDialer) Dial() (net.Conn, error) { | |
| return net.DialTimeout("unix", d.socketPath, d.timeout) |
|
|
||
| //nolint:staticcheck | ||
| l := libvirt.New(c) | ||
| _ = c.Close() | ||
| l := libvirt.NewWithDialer(&libvirtUnixDialer{uri: uri, timeout: 2 * time.Second}) |
There was a problem hiding this comment.
The code now dials c := net.DialTimeout(...) and then immediately closes it (_ = c.Close()), but the real libvirt client is created with NewWithDialer(...) which will dial again. This introduces an extra connection attempt that wasn’t present when libvirt.New(c) reused the already-open connection. Consider removing the probe dial entirely (let ConnectToURI surface the dial error), or move the system/session fallback logic into the dialer so only one dial happens.
The fast path using binary.LittleEndian.Uint32 % uint32(max) triggers gosec G115 (integer overflow conversion int64 -> uint32) on the uint32(max) cast, even though max <= 1<<31-1 fits in uint32. Reverting to the simple rand.Int approach which avoids the issue cleanly. Issue: PR poyrazK#154 review item #2
Summary\n- resolve staticcheck S1021 in execution ledger by merging declaration/assignment\n- migrate deprecated libvirt constructor usage to with a local unix dialer\n- suppress generated protobuf SA1019 deprecation warnings with file-level staticcheck ignore\n\n## Validation\n- ok github.com/poyrazk/thecloud/internal/repositories/postgres (cached)
ok github.com/poyrazk/thecloud/internal/repositories/libvirt 0.401s
? github.com/poyrazk/thecloud/internal/autoscaler/protos [no test files]\n- \n\nThis PR is intentionally scoped to staticcheck cleanup only.