Skip to content
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

Merged
merged 4 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,6 @@ linters:
- gosec
- revive
- misspell
- gocritic
disable:
- errcheck
4 changes: 2 additions & 2 deletions hack/verify-docs.sh
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
set -eu -o pipefail

if ( git diff --name-only |grep -q '^docs\/' ) ; then
echo "[ERROR] Markdown documenation is out of sync, run 'make generate-docs' and commit the changes!"
echo "[ERROR] Markdown documentation is out of sync, run 'make generate-docs' and commit the changes!"
exit 1
fi
fi
3 changes: 1 addition & 2 deletions pkg/shp/cmd/buildrun/logs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...) {
Copy link
Contributor Author

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

Copy link
Contributor

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.

logs, err := util.GetPodLogs(c.cmd.Context(), clientset, pod, container.Name)
if err != nil {
return err
Expand Down
3 changes: 1 addition & 2 deletions pkg/shp/cmd/follower/follow.go
Original file line number Diff line number Diff line change
Expand Up @@ -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...) {
Copy link
Contributor Author

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

if _, exists := f.tailLogsStarted[container.Name]; exists {
continue
}
Expand Down
4 changes: 1 addition & 3 deletions pkg/shp/reactor/pod_watcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably unused by now.

p.eventTicker.Stop()
switch event.Type {
case watch.Added:
Expand Down Expand Up @@ -235,6 +233,6 @@ func NewPodWatcher(
clientset kubernetes.Interface,
ns string,
) (*PodWatcher, error) {
//TODO don't think the have not received events yet ticker needs to be tunable, but leaving a TODO for now while we get feedback
// TODO don't think the have not received events yet ticker needs to be tunable, but leaving a TODO for now while we get feedback
return &PodWatcher{ctx: ctx, to: timeout, ns: ns, clientset: clientset, eventTicker: time.NewTicker(1 * time.Second), stopCh: make(chan bool), stopLock: sync.Mutex{}}, nil
}
2 changes: 1 addition & 1 deletion pkg/shp/streamer/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor Author

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)`

}

func writeFileToTar(tw *tar.Writer, src, fpath string, stat fs.FileInfo) error {
Expand Down
6 changes: 3 additions & 3 deletions test/e2e/envvars.bats
Original file line number Diff line number Diff line change
Expand Up @@ -44,15 +44,15 @@ teardown() {
run kubectl get buildruns.shipwright.io/${buildrun_name} -o yaml
assert_success


# ensure that the environment variables were inserted into the Build object
assert_output --partial "VAR_2" && assert_output --partial "buildrun-value-2"
assert_output --partial "VAR_3" && assert_output --partial "buildrun-value-3"

# get the taskrun that we created
run kubectl get taskruns.tekton.dev --selector=buildrun.shipwright.io/name=${buildrun_name} -o name
run kubectl get taskruns.tekton.dev --selector=buildrun.shipwright.io/name=${buildrun_name} -o name
assert_success

run kubectl get ${output} -o yaml
assert_success

Expand Down
Loading