-
Notifications
You must be signed in to change notification settings - Fork 28
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
Fix gocritic
findings
#292
Conversation
@@ -110,8 +110,7 @@ func (c *LogsCommand) Run(params *params.Params, ioStreams *genericclioptions.IO | |||
fmt.Fprintf(ioStreams.Out, "Obtaining logs for BuildRun %q\n\n", c.name) | |||
|
|||
var b strings.Builder | |||
containers := append(pod.Spec.InitContainers, pod.Spec.Containers...) | |||
for _, container := range containers { | |||
for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) { |
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.
./pkg/shp/cmd/buildrun/logs.go:113:17: appendAssign: append result not assigned to the same slice
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.
never saw such range with append on the same line, nice.
@@ -107,8 +107,7 @@ func (f *Follower) Log(msg string) { | |||
// tailLogs start tailing logs for each container name in init-containers and containers, if not | |||
// started already. | |||
func (f *Follower) tailLogs(pod *corev1.Pod) { | |||
containers := append(pod.Spec.InitContainers, pod.Spec.Containers...) | |||
for _, container := range containers { | |||
for _, container := range append(pod.Spec.InitContainers, pod.Spec.Containers...) { |
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.
./pkg/shp/cmd/follower/follow.go:110:16: appendAssign: append result not assigned to the same slice
@@ -94,8 +94,6 @@ func (p *PodWatcher) WithNoPodEventsYetFn(fn NoPodEventsYetFn) *PodWatcher { | |||
|
|||
// handleEvent applies user informed functions against informed pod and event. | |||
func (p *PodWatcher) handleEvent(pod *corev1.Pod, event watch.Event) error { | |||
//p.stopLock.Lock() | |||
//defer p.stopLock.Unlock() |
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.
Probably unused by now.
@@ -18,7 +18,7 @@ func (wc *writeCounter) Write(p []byte) (int, error) { | |||
} | |||
|
|||
func trimPrefix(prefix, fpath string) string { | |||
return strings.TrimPrefix(strings.Replace(fpath, prefix, "", -1), string(filepath.Separator)) | |||
return strings.TrimPrefix(strings.ReplaceAll(fpath, prefix, ""), string(filepath.Separator)) |
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.
./pkg/shp/streamer/util.go:21:28: wrapperFunc: use strings.ReplaceAll method in `strings.Replace(fpath, prefix, "", -1)`
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.
/lgtm
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.
Thanks, looks good. Do you mind enabling the linter in .golangci.yml
. Otherwise it won't take too long for new findings.
Resolve `gocritic` findings. Signed-off-by: Matthias Diester <matthias.diester@de.ibm.com>
Remove trailing whitespaces. Signed-off-by: Matthias Diester <matthias.diester@de.ibm.com>
d942093
to
514deb4
Compare
Add `gocritic` to the list of default linters. Signed-off-by: Matthias Diester <matthias.diester@de.ibm.com>
514deb4
to
32b23a6
Compare
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.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: SaschaSchwarze0 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Changes
While working on #291, I realized that my linter returned some minor findings in this repository.
Fix minor TYPO in
hack/verify-docs.sh
.Resolve
gocritic
findings.Remove trailing whitespaces.
Submitter Checklist
See the contributor guide
for details on coding conventions, github and prow interactions, and the code review process.
Release Notes