-
Notifications
You must be signed in to change notification settings - Fork 23
common: fix linter #262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
common: fix linter #262
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses linter warnings (specifically the prealloc warning) for v1.7 by refactoring slice allocations to use proper pre-allocation instead of suppressing warnings with nolint comments.
Changes:
- Replaced slice literal assignments with pre-allocated slices using
make()with appropriate capacity - Removed
nolint: prealloccomments that were suppressing linter warnings - Removed unused
path/filepathimport after switching fromfilepath.Jointo string concatenation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/utils/command.go | Pre-allocated cmdArgs slice and replaced filepath.Join with string concatenation for namespace paths |
| cmd/node-disk-manager-webhook/main.go | Pre-allocated starters slice with capacity 4 and moved declaration after error checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pkg/utils/command.go
Outdated
| "--mount="+exec.namespace+"/mnt", | ||
| "--net="+exec.namespace+"/net", | ||
| "--ipc="+exec.namespace+"/ipc", |
Copilot
AI
Jan 22, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replacing filepath.Join with string concatenation introduces a subtle change in path construction. Since exec.namespace comes from GetHostNamespacePath which returns paths ending with "/" (e.g., "/proc/1/ns/"), the new code produces paths like "/proc/1/ns//mnt" with double slashes, whereas filepath.Join would have produced "/proc/1/ns/mnt". While Linux generally tolerates double slashes, this behavioral change is unintended. To properly address the linter warning while maintaining exact behavior, consider using pre-allocated slices with filepath.Join, or ensure exec.namespace is trimmed of trailing slashes before concatenation.
Signed-off-by: Vicente Cheng <vicente.cheng@suse.com>
4028c3c to
586f9d6
Compare
|
@Mergifyio backport v1.7 |
✅ Backports have been createdDetails
Cherry-pick of a4796fd has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Problem:
fix linter for v1.7
Solution:
fix linter for v1.7
Related Issue:
Test plan: