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.

Sign up for GitHub

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

Jump to bottom

chore(lint): Enable linting with gosimple #26965

Merged
merged 1 commit into from
Jul 28, 2023
Merged

chore(lint): Enable linting with gosimple #26965

merged 1 commit into from
Jul 28, 2023

Conversation

mrueg
Copy link
Contributor

@mrueg mrueg commented Jul 20, 2023 •
edited
Loading

This fixes issues found by the gosimple linter and enables it in the golangci config. gosimple is enabled by default in golang-ci.
Unfortunately it touches a lot of different code, please let me know if you want to have the PR broken down per file path.

Please ensure your pull request adheres to the following guidelines:

  • For first time contributors, read Submitting a pull request
  • All code is covered by unit and/or runtime tests where feasible.
  • All commits contain a well written commit description including a title,
    description and a Fixes: #XXX line if the commit addresses a particular
    GitHub issue.
  • If your commit description contains a Fixes: <commit-id> tag, then
    please add the commit author[s] as reviewer[s] to this issue.
  • All commits are signed off. See the section Developer’s Certificate of Origin
  • Provide a title or release-note blurb suitable for the release notes.
  • Are you a user of Cilium? Please add yourself to the Users doc
  • Thanks for contributing!
bugtool/cmd/root.go:455:4: S1038: should use fmt.Fprintf instead of fmt.Fprint(fmt.Sprintf(...)) (gosimple)
                        fmt.Fprint(f, fmt.Sprintf("# %s\n\n```\n%s\n```\n", prompt, output))
                        ^
pkg/hive/job/job.go:318:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/hive/job/job.go:543:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/k8s/resource/resource.go:210:5: S1002: should omit comparison to bool constant, can be simplified to `!status` (gosimple)
        if status == false {
           ^
operator/pkg/model/translation/envoy_virtual_host.go:99:2: S1008: should use 'return headerMatch1 > headerMatch2' instead of 'if headerMatch1 > headerMatch2 { return true }; return false' (gosimple)
        if headerMatch1 > headerMatch2 {
        ^
pkg/maps/ipmasq/ipmasq.go:30:42: S1025: should use String() instead of fmt.Sprintf (gosimple)
func (k *Key4) String() string  { return fmt.Sprintf("%s", k.Address) }
                                         ^
pkg/maps/ipmasq/ipmasq.go:38:42: S1025: should use String() instead of fmt.Sprintf (gosimple)
func (k *Key6) String() string  { return fmt.Sprintf("%s", k.Address) }
                                         ^
test/helpers/kubectl.go:4391:10: S1039: unnecessary use of fmt.Sprintf (gosimple)
                cmd := fmt.Sprintf(`cilium bpf egress list | tail -n +2 | wc -l`)
                       ^
test/helpers/utils.go:468:6: S1005: unnecessary assignment to the blank identifier (gosimple)
                                        count, _ := uniqueFailures[msg]
                                        ^
test/helpers/kubectl.go:939:55: S1025: the argument is already a string, there's no need to use fmt.Sprintf (gosimple)
        podsNodes, err := kub.GetPodsNodes(DefaultNamespace, fmt.Sprintf("%s", podFilter))
                                                             ^
test/helpers/cilium.go:420:5: S1002: should omit comparison to bool constant, can be simplified to `!data.WasSuccessful()` (gosimple)
        if data.WasSuccessful() == false {
           ^
test/helpers/cilium.go:594:5: S1002: should omit comparison to bool constant, can be simplified to `!res.WasSuccessful()` (gosimple)
        if res.WasSuccessful() == false {
           ^
test/helpers/cilium.go:602:5: S1002: should omit comparison to bool constant, can be simplified to `!res.WasSuccessful()` (gosimple)
        if res.WasSuccessful() == false {
           ^
test/helpers/cilium.go:884:5: S1002: should omit comparison to bool constant, can be simplified to `!ok` (gosimple)
        if ok == false {
           ^
test/helpers/cilium.go:902:6: S1002: should omit comparison to bool constant, can be simplified to `!result` (gosimple)
                if result == false {
                   ^
test/helpers/kubectl.go:277:5: S1002: should omit comparison to bool constant, can be simplified to `!config.CiliumTestConfig.ProvisionK8s` (gosimple)
        if config.CiliumTestConfig.ProvisionK8s == false {
           ^
test/helpers/kubectl.go:2826:7: S1002: should omit comparison to bool constant, can be simplified to `!status` (gosimple)
                        if status == false {
                           ^
test/helpers/cilium.go:763:2: S1000: should use for range instead of for { select {} } (gosimple)
        for {
        ^
test/helpers/kubectl.go:1319:2: S1000: should use for range instead of for { select {} } (gosimple)
        for {
        ^
test/helpers/ssh_command.go:219:3: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
                select {
                ^
test/helpers/utils.go:157:4: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
                        select {
                        ^
test/helpers/kubectl.go:2922:4: S1023: redundant break statement (gosimple)
                        break
                        ^
test/helpers/cilium.go:248:3: S1008: should use 'return res.WasSuccessful()' instead of 'if !res.WasSuccessful() { return false }; return true' (gosimple)
                if !res.WasSuccessful() {
                ^
test/helpers/cilium.go:315:3: S1008: should use 'return result[desiredState] == total' instead of 'if result[desiredState] == total { return true }; return false' (gosimple)
                if result[desiredState] == total {
                ^
test/helpers/utils.go:820:2: S1008: should use 'return os.Getenv("CILIUM_NO_IPV6_OUTSIDE") == ""' instead of 'if os.Getenv("CILIUM_NO_IPV6_OUTSIDE") != "" { return false }; return true' (gosimple)
        if os.Getenv("CILIUM_NO_IPV6_OUTSIDE") != "" {
        ^
tools/crdcheck/main.go:26:3: S1038: should use log.Fatalf(...) instead of log.Fatal(fmt.Sprintf(...)) (gosimple)
                log.Fatal(fmt.Sprintf("usage: %s <path>", os.Args[0]))
                ^
operator/pkg/lbipam/lbipam_test.go:133:3: S1008: should use 'return isPoolConflicting(pool)' instead of 'if !isPoolConflicting(pool) { return false }; return true' (gosimple)
                if !isPoolConflicting(pool) {
                ^
operator/pkg/lbipam/lbipam_test.go:154:3: S1008: should use 'return !isPoolConflicting(pool)' instead of 'if isPoolConflicting(pool) { return false }; return true' (gosimple)
                if isPoolConflicting(pool) {
                ^
pkg/ip/ip.go:865:2: S1001: should use copy(to, from) instead of a loop (gosimple)
        for i := 0; i < len(ipList); i++ {
        ^
test/k8s/services.go:574:34: S1039: unnecessary use of fmt.Sprintf (gosimple)
                                "devices":                   fmt.Sprintf(`'{}'`), // Revert back to auto-detection after XDP.
                                                             ^
test/k8s/services.go:588:34: S1039: unnecessary use of fmt.Sprintf (gosimple)
                                "devices":                   fmt.Sprintf(`'{}'`), // Revert back to auto-detection after XDP.
                                                             ^
test/k8s/services.go:601:34: S1039: unnecessary use of fmt.Sprintf (gosimple)
                                "devices":                   fmt.Sprintf(`'{}'`),
                                                             ^
test/k8s/services.go:614:34: S1039: unnecessary use of fmt.Sprintf (gosimple)
                                "devices":                   fmt.Sprintf(`'{}'`), // Revert back to auto-detection after XDP.
                                                             ^
test/k8s/services.go:626:34: S1039: unnecessary use of fmt.Sprintf (gosimple)
                                "devices":                   fmt.Sprintf(`'{}'`),
                                                             ^
test/k8s/kafka_policies.go:72:5: S1008: should use 'return err == nil' instead of 'if err != nil { return false }; return true' (gosimple)
                                if err != nil {
                                ^
test/k8s/kafka_policies.go:86:5: S1008: should use 'return res.WasSuccessful()' instead of 'if !res.WasSuccessful() { return false }; return true' (gosimple)
                                if !res.WasSuccessful() {
                                ^
test/k8s/kafka_policies.go:156:6: S1008: should use 'return errBody == nil' instead of 'if errBody != nil { return false }; return true' (gosimple)
                                        if errBody != nil {
                                        ^
pkg/stream/operators_test.go:175:11: S1012: should use `time.Since` instead of `time.Now().Sub` (gosimple)
        tdiff := time.Now().Sub(t0)
                 ^
pkg/monitor/datapath_debug.go:543:3: S1038: should use fmt.Printf instead of fmt.Println(fmt.Sprintf(...)) (but don't forget the newline) (gosimple)
                fmt.Println(fmt.Sprintf(`{"type":"debug_capture_error","message":%q}`, err.Error()))
                ^
pkg/node/manager/manager.go:702:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/l2announcer/l2announcer.go:169:3: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
                select {
                ^
pkg/k8s/slim/k8s/apis/labels/selector.go:911:5: S1009: should omit nil check; len() for github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/labels.Set is defined as zero (gosimple)
        if ls == nil || len(ls) == 0 {
           ^
pkg/k8s/slim/k8s/apis/labels/selector.go:933:5: S1009: should omit nil check; len() for github.com/cilium/cilium/pkg/k8s/slim/k8s/apis/labels.Set is defined as zero (gosimple)
        if ls == nil || len(ls) == 0 {
           ^
pkg/bgpv1/gobgp/conversions.go:65:19: S1012: should use `time.Since` instead of `time.Now().Sub` (gosimple)
        ageNano := int64(time.Now().Sub(p.Age.AsTime()))
                         ^
pkg/bgpv1/gobgp/state.go:71:41: S1012: should use `time.Since` instead of `time.Now().Sub` (gosimple)
                                peerState.UptimeNanoseconds = int64(time.Now().Sub(peer.Timers.State.Uptime.AsTime()))
                                                                    ^
pkg/egressgateway/manager.go:306:4: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
                        select {
                        ^
pkg/egressgateway/manager_privileged_test.go:707:4: S1023: redundant `return` statement (gosimple)
                        return
                        ^
pkg/proxy/proxy.go:392:27: S1002: should omit comparison to bool constant, can be simplified to `!p.allocatedPorts[pp.rulesPort]` (gosimple)
                if pp.rulesPort != 0 && p.allocatedPorts[pp.rulesPort] == false {
                                        ^
pkg/fqdn/proxy/proxy.go:29:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/fqdn/proxy/proxy.go:41:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/fqdn/proxy/proxy.go:45:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/fqdn/proxy/proxy.go:49:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/datapath/loader/cache.go:366:8: S1005: unnecessary assignment to the blank identifier (gosimple)
                case err, _ = <-templateWatcher.Errors:
                     ^
operator/pkg/ciliumenvoyconfig/envoy_config.go:219:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var filterChains []*envoy_config_listener.FilterChain
        ^
pkg/datapath/linux/node.go:648:4: S1012: should use `time.Since` instead of `time.Now().Sub` (gosimple)
                        time.Now().Sub(lastPing) < option.Config.ARPPingRefreshPeriod {
                        ^
pkg/datapath/linux/node.go:2039:5: S1002: should omit comparison to bool constant, can be simplified to `found` (gosimple)
        if found == true {
           ^
pkg/fqdn/name_manager_test.go:55:2: S1005: unnecessary assignment to the blank identifier (gosimple)
        ips, _ := selIPMap[ciliumIOSel]
        ^
pkg/fqdn/cache.go:945:4: S1033: unnecessary guard around call to delete (gosimple)
                        if _, ok := possibleAlive[dead]; ok {
                        ^
pkg/datapath/fake/node.go:65:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/datapath/fake/node.go:69:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/datapath/fake/node.go:85:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/monitor/format/fuzz_test.go:44:3: S1023: redundant `return` statement (gosimple)
                return
                ^
pkg/service/service_test.go:912:2: S1005: unnecessary assignment to the blank identifier (gosimple)
        svc, _ = m.svc.svcByID[id]
        ^
pkg/service/service.go:1264:5: S1011: should replace loop with `toDeleteAffinity = append(toDeleteAffinity, obsoleteSVCBackendIDs...)` (gosimple)
                                for _, bID := range obsoleteSVCBackendIDs {
                                ^
pkg/ipam/allocator/podcidr/podcidr.go:413:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/ipam/allocator/podcidr/podcidr_test.go:225:7: S1023: redundant `return` statement (gosimple)
                                                return
                                                ^
pkg/ipam/allocator/podcidr/podcidr_test.go:309:7: S1023: redundant `return` statement (gosimple)
                                                return
                                                ^
pkg/ipam/allocator/podcidr/podcidr_test.go:371:7: S1023: redundant `return` statement (gosimple)
                                                return
                                                ^
pkg/ipam/allocator/podcidr/podcidr_test.go:427:7: S1023: redundant `return` statement (gosimple)
                                                return
                                                ^
pkg/ipam/allocator/podcidr/podcidr_test.go:518:7: S1023: redundant `return` statement (gosimple)
                                                return
                                                ^
operator/pkg/ciliumendpointslice/manager.go:587:2: S1005: unnecessary assignment to the blank identifier (gosimple)
        identity, _ := c.cesToIdentity[cesName]
        ^
operator/pkg/ciliumendpointslice/manager.go:179:2: S1033: unnecessary guard around call to delete (gosimple)
        if _, ok := ces.removedCEPs[GetCEPNameFromCCEP(cep, ces.ces.Namespace)]; ok {
        ^
operator/pkg/ciliumendpointslice/endpointslice.go:190:2: S1023: redundant `return` statement (gosimple)
        return
        ^
operator/pkg/ciliumendpointslice/endpointslice.go:277:2: S1023: redundant `return` statement (gosimple)
        return
        ^
operator/pkg/ciliumendpointslice/manager.go:185:2: S1023: redundant `return` statement (gosimple)
        return
        ^
operator/pkg/ciliumendpointslice/manager.go:374:2: S1023: redundant `return` statement (gosimple)
        return
        ^
operator/pkg/ciliumendpointslice/endpointslice_test.go:284:3: S1021: should merge variable declaration with assignment on next line (gosimple)
                var err error
                ^
pkg/datapath/iptables/iptables.go:1623:5: S1002: should omit comparison to bool constant, can be simplified to `!option.Config.EnableIPSec` (gosimple)
        if option.Config.EnableIPSec == false {
           ^
pkg/datapath/iptables/iptables.go:126:7: S1007: should use raw string (`...`) with regexp.MustCompile to avoid having to escape twice (gosimple)
        v := regexp.MustCompile("v([0-9]+(\\.[0-9]+)+)")
             ^
pkg/identity/cache/cache.go:144:4: S1000: should use for range instead of for { select {} } (gosimple)
                        for {
                        ^
test/ginkgo-ext/scopes.go:304:2: S1005: unnecessary assignment to the blank identifier (gosimple)
        hasFailed, _ := afterEachFailed[testName]
        ^
test/ginkgo-ext/scopes.go:371:5: S1002: should omit comparison to bool constant, can be simplified to `!hasFailed` (gosimple)
        if hasFailed == false && afterEachFailed[testName] {
           ^
pkg/maps/ctmap/per_cluster_ctmap.go:475:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/k8s/service.go:665:3: S1002: should omit comparison to bool constant, can be simplified to `!s.SessionAffinity` (gosimple)
                s.SessionAffinity == false &&
                ^
pkg/k8s/labels.go:124:3: S1011: should replace loop with `containerPorts = append(containerPorts, containers.Ports...)` (gosimple)
                for _, cp := range containers.Ports {
                ^
pkg/k8s/service.go:114:3: S1023: redundant break statement (gosimple)
                break
                ^
pkg/k8s/service.go:118:3: S1023: redundant break statement (gosimple)
                break
                ^
pkg/k8s/service.go:122:3: S1023: redundant break statement (gosimple)
                break
                ^
pkg/k8s/cnpstatus.go:117:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/k8s/endpoints.go:80:2: S1023: redundant `return` statement (gosimple)
        return
        ^
cilium/cmd/bpf_sha_get.go:52:34: S1025: the argument's underlying type is a slice of bytes, should use a simple conversion instead of fmt.Sprintf (gosimple)
                jsonEncStr := regex.FindString(fmt.Sprintf("%s", text))
                                               ^
cilium/cmd/prefilter_list.go:51:9: S1025: the argument is already a string, there's no need to use fmt.Sprintf (gosimple)
                str = fmt.Sprintf("%s", pfx)
                      ^
cilium/cmd/cleanup.go:283:5: S1002: should omit comparison to bool constant, can be simplified to `!(cleanAll || cleanBPF)` (gosimple)
        if (cleanAll || cleanBPF) == false {
           ^
cilium/cmd/debuginfo.go:367:2: S1023: redundant `return` statement (gosimple)
        return
        ^
cilium/cmd/lrp_list.go:64:4: S1038: should use fmt.Fprintf instead of fmt.Fprintln(fmt.Sprintf(...)) (but don't forget the newline) (gosimple)
                        fmt.Fprintln(w, fmt.Sprintf("\t|\t%s", getPrintableMapping(feM)))
                        ^
pkg/alibabacloud/api/api.go:222:3: S1011: should replace loop with `result = append(result, resp.InstanceTypes.InstanceType...)` (gosimple)
                for _, v := range resp.InstanceTypes.InstanceType {
                ^
pkg/alibabacloud/api/api.go:415:3: S1011: should replace loop with `result = append(result, resp.NetworkInterfaceSets.NetworkInterfaceSet...)` (gosimple)
                for _, v := range resp.NetworkInterfaceSets.NetworkInterfaceSet {
                ^
pkg/alibabacloud/api/api.go:445:3: S1011: should replace loop with `result = append(result, resp.NetworkInterfaceSets.NetworkInterfaceSet...)` (gosimple)
                for _, v := range resp.NetworkInterfaceSets.NetworkInterfaceSet {
                ^
pkg/datapath/prefilter/prefilter.go:149:6: S1002: should omit comparison to bool constant, can be simplified to `!p.maps[which].CIDRExists(cidr)` (gosimple)
                if p.maps[which].CIDRExists(cidr) == false {
                   ^
operator/watchers/k8s_service_sync.go:111:38: S1019: should use make(chan struct{}) instead (gosimple)
        kvstoreReady := make(chan struct{}, 0)
                                            ^
pkg/hubble/container/ring.go:127:32: S1019: should use make([]*v1.Event, dataLen) instead (gosimple)
                data:      make([]*v1.Event, dataLen, dataLen),
                                             ^
pkg/hubble/container/ring_test.go:41:25: S1019: should use make([]*v1.Event, b.N) instead (gosimple)
        a := make([]*v1.Event, b.N, b.N)
                               ^
pkg/hubble/container/ring_test.go:57:25: S1019: should use make([]*v1.Event, b.N) instead (gosimple)
        a := make([]*v1.Event, b.N, b.N)
                               ^
pkg/hubble/container/ring_test.go:72:25: S1019: should use make([]*v1.Event, b.N) instead (gosimple)
        a := make([]*v1.Event, b.N, b.N)
                               ^
pkg/hubble/parser/threefour/parser.go:100:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var eventType uint8
        ^
pkg/alibabacloud/eni/node.go:91:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/datapath/tables/l2_announce.go:29:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var n L2AnnounceEntry
        ^
pkg/k8s/watchers/watcher.go:987:5: S1002: should omit comparison to bool constant, can be simplified to `!status` (gosimple)
        if status == false {
           ^
pkg/k8s/watchers/node.go:37:2: S1008: should use 'return comparator.MapStringEquals(oldNode.GetLabels(), newNode.GetLabels())' instead of 'if !comparator.MapStringEquals(oldNode.GetLabels(), newNode.GetLabels()) { return false }; return true' (gosimple)
        if !comparator.MapStringEquals(oldNode.GetLabels(), newNode.GetLabels()) {
        ^
pkg/kvstore/etcd_lease_test.go:128:34: S1019: should use make(chan client.LeaseID) instead (gosimple)
        ch := make(chan client.LeaseID, 0)
                                        ^
pkg/cidr/cidr.go:75:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/comparator/comparator.go:105:2: S1008: should use 'return len(m1)-ignoredM1 == len(m2)-ignoredM2' instead of 'if len(m1)-ignoredM1 != len(m2)-ignoredM2 { return false }; return true' (gosimple)
        if len(m1)-ignoredM1 != len(m2)-ignoredM2 {
        ^
plugins/cilium-docker/driver/ipam.go:46:5: S1002: should omit comparison to bool constant, can be simplified to `!req.V6` (gosimple)
        if req.V6 == false {
           ^
pkg/cgroups/manager/manager.go:135:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
        select {
        ^
pkg/cgroups/manager/manager.go:152:2: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
        select {
        ^
daemon/cmd/kube_proxy_replacement.go:441:11: S1039: unnecessary use of fmt.Sprintf (gosimple)
                                msg = fmt.Sprintf("BPF host routing requires kernel 5.10 or newer.")
                                      ^
daemon/cmd/debuginfo.go:29:22: S1025: should use String() instead of fmt.Sprintf (gosimple)
                dr.KernelVersion = fmt.Sprintf("%s", kver)
                                   ^
daemon/cmd/status.go:770:12: S1025: the argument is already a string, there's no need to use fmt.Sprintf (gosimple)
                                        msg = fmt.Sprintf("%s", info)
                                              ^
daemon/cmd/state.go:252:2: S1031: unnecessary nil check around range (gosimple)
        if existingEndpoints != nil {
        ^
daemon/cmd/config.go:112:2: S1023: redundant `return` statement (gosimple)
        return
        ^
daemon/cmd/policy.go:397:2: S1023: redundant `return` statement (gosimple)
        return
        ^
daemon/cmd/policy.go:620:2: S1023: redundant `return` statement (gosimple)
        return
        ^
daemon/cmd/status.go:525:2: S1023: redundant `return` statement (gosimple)
        return
        ^
daemon/cmd/status.go:530:2: S1023: redundant `return` statement (gosimple)
        return
        ^
daemon/cmd/status.go:550:2: S1023: redundant `return` statement (gosimple)
        return
        ^
daemon/cmd/status.go:1078:2: S1023: redundant `return` statement (gosimple)
        return
        ^
daemon/cmd/state.go:268:40: S1019: should use make(chan struct{}) instead (gosimple)
        restoreComplete = make(chan struct{}, 0)
                                              ^
pkg/testutils/privileged.go:22:3: S1038: should use tb.Skipf(...) instead of tb.Skip(fmt.Sprintf(...)) (gosimple)
                tb.Skip(fmt.Sprintf("Set %s to run this test", privilegedEnv))
                ^
pkg/testutils/privileged.go:39:3: S1038: should use tb.Skipf(...) instead of tb.Skip(fmt.Sprintf(...)) (gosimple)
                tb.Skip(fmt.Sprintf("Set %s to run this test", integrationEnv))
                ^
pkg/testutils/privileged.go:47:3: S1038: should use tb.Skipf(...) instead of tb.Skip(fmt.Sprintf(...)) (gosimple)
                tb.Skip(fmt.Sprintf("Set %s to run this test", gatewayAPIConformanceEnv))
                ^
pkg/testutils/privileged.go:28:2: S1008: should use 'return os.Getenv(integrationEnv) != ""' instead of 'if os.Getenv(integrationEnv) != "" { return true }; return false' (gosimple)
        if os.Getenv(integrationEnv) != "" {
        ^
pkg/bgpv1/test/neighbor_test.go:158:19: S1024: should use time.Until instead of t.Sub(time.Now()) (gosimple)
                        outstanding := deadline.Sub(time.Now())
                                       ^
pkg/bgpv1/test/neighbor_test.go:301:19: S1024: should use time.Until instead of t.Sub(time.Now()) (gosimple)
                        outstanding := deadline.Sub(time.Now())
                                       ^
operator/pkg/ingress/ingress_class.go:165:22: S1016: should convert event (type ingressClassDeletedEvent) to ciliumIngressClassDeletedEvent instead of using struct literal (gosimple)
                i.ingressQueue.Add(ciliumIngressClassDeletedEvent{ingressClass: event.ingressClass})
                                   ^
pkg/redirectpolicy/manager_test.go:56:30: S1019: should use make([]interface{}, 2) instead (gosimple)
        pods := make([]interface{}, 2, 2)
                                    ^
pkg/endpoint/status.go:58:10: S1025: should use String() instead of fmt.Sprintf (gosimple)
                return fmt.Sprintf("%s", s.Code)
                       ^
pkg/endpoint/api.go:505:28: S1004: should use !bytes.Equal(e.mac, newEp.mac) instead (gosimple)
        if len(newEp.mac) != 0 && bytes.Compare(e.mac, newEp.mac) != 0 {
                                  ^
pkg/endpoint/api.go:510:32: S1004: should use !bytes.Equal(e.GetNodeMAC(), newEp.nodeMAC) instead (gosimple)
        if len(newEp.nodeMAC) != 0 && bytes.Compare(e.GetNodeMAC(), newEp.nodeMAC) != 0 {
                                      ^
pkg/endpoint/policy.go:676:3: S1000: should use a simple channel send/receive instead of `select` with a single case (gosimple)
                select {
                ^
pkg/endpoint/events.go:68:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/endpoint/events.go:185:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/endpoint/events.go:251:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/endpoint/redirect_test.go:78:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/endpoint/endpoint.go:1755:2: S1008: should use 'return myChangeRev != e.identityRevision' instead of 'if myChangeRev != e.identityRevision { return true }; return false' (gosimple)
        if myChangeRev != e.identityRevision {
        ^
pkg/endpoint/endpoint.go:477:41: S1019: should use make(chan struct{}) instead (gosimple)
                hasBPFProgram:    make(chan struct{}, 0),
                                                      ^
pkg/endpoint/endpoint.go:843:41: S1019: should use make(chan struct{}) instead (gosimple)
        ep.hasBPFProgram = make(chan struct{}, 0)
                                               ^
pkg/aws/eni/node.go:114:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/maps/srv6map/policy.go:54:9: S1025: should use String() instead of fmt.Sprintf (gosimple)
        return fmt.Sprintf("%s", v.SID)
               ^
pkg/maps/srv6map/sid.go:30:9: S1025: should use String() instead of fmt.Sprintf (gosimple)
        return fmt.Sprintf("%s", k.SID)
               ^
pkg/types/ipv4.go:33:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/types/ipv6.go:29:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/types/macaddr.go:24:2: S1023: redundant `return` statement (gosimple)
        return
        ^
pkg/types/ipv4_test.go:28:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var expectedAddress net.IP
        ^
pkg/types/ipv6_test.go:22:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var expectedAddress net.IP
        ^
pkg/types/macaddr_test.go:21:2: S1021: should merge variable declaration with assignment on next line (gosimple)
        var expectedAddress net.HardwareAddr
        ^
operator/pkg/gateway-api/controller_test.go:336:27: S1039: unnecessary use of fmt.Sprintf (gosimple)
                        if !tt.wantErr(t, err, fmt.Sprintf("success()")) {
                                               ^
pkg/datapath/linux/ipsec/ipsec_linux.go:136:3: S1005: unnecessary assignment to the blank identifier (gosimple)
                key, _ = ipSecKeysGlobal[""]
                ^
pkg/datapath/linux/ipsec/ipsec_linux.go:135:5: S1002: should omit comparison to bool constant, can be simplified to `!scoped` (gosimple)
        if scoped == false {
           ^

chore(lint): Enable linting with gosimple

@mrueg mrueg requested review from a team as code owners July 20, 2023 18:02
@maintainer-s-little-helper maintainer-s-little-helper bot added the dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. label Jul 20, 2023
@christarazi christarazi added kind/cleanup This includes no functional changes. area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. area/CI-improvement Topic or proposal to improve the Continuous Integration workflow release-note/misc This PR makes changes that have no direct user impact. area/build Anything to do with the build, more general than area/CI and removed dont-merge/needs-release-note-label The author needs to describe the release impact of these changes. labels Jul 20, 2023
chancez
chancez approved these changes Jul 21, 2023
View reviewed changes
Copy link
Contributor

@chancez chancez left a comment

Choose a reason for hiding this comment

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

Hubble files LGTM. Minor improvement suggested but non-blocking.

@mrueg
Copy link
Contributor Author

mrueg commented Jul 21, 2023

Hubble files LGTM. Minor improvement suggested but non-blocking.

I can apply that, will it reset the approvals though?

@chancez
Copy link
Contributor

chancez commented Jul 21, 2023

Hubble files LGTM. Minor improvement suggested but non-blocking.

I can apply that, will it reset the approvals though?

No, but it will rerun tests.

@christarazi
Copy link
Member

/test

pippolo84
pippolo84 approved these changes Jul 24, 2023
View reviewed changes
Copy link
Member

@pippolo84 pippolo84 left a comment

Choose a reason for hiding this comment

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

Thanks 🚀

@mrueg mrueg force-pushed the lint-gosimple branch 5 times, most recently from 6d9e140 to c761b46 Compare July 26, 2023 15:44
@mrueg
chore(lint): Enable linting with gosimple
863cbc9 
This fixes issues found by the gosimple linter and enables it in the
golangci config.

Signed-off-by: Manuel Rüger <manuel@rueg.eu>
@youngnick youngnick removed the request for review from aditighag July 26, 2023 23:58
@mhofstetter
Copy link
Member

/test

@maintainer-s-little-helper maintainer-s-little-helper bot added the ready-to-merge This PR has passed all tests and received consensus from code owners to merge. label Jul 27, 2023
@youngnick youngnick merged commit 61477fc into cilium:main Jul 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gentoo-root gentoo-root gentoo-root approved these changes

@chancez chancez chancez approved these changes

@pippolo84 pippolo84 pippolo84 approved these changes

@christarazi christarazi christarazi approved these changes

@aanm aanm aanm approved these changes

@mhofstetter mhofstetter mhofstetter approved these changes

@youngnick youngnick youngnick approved these changes

@rastislavs rastislavs rastislavs approved these changes

@rgo3 rgo3 rgo3 approved these changes

Assignees
No one assigned
Labels
area/build Anything to do with the build, more general than area/CI area/CI-improvement Topic or proposal to improve the Continuous Integration workflow area/misc Impacts miscellaneous areas of the code not otherwise owned by another area. kind/cleanup This includes no functional changes. kind/community-contribution This was a contribution made by a community member. ready-to-merge This PR has passed all tests and received consensus from code owners to merge. release-note/misc This PR makes changes that have no direct user impact.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@mrueg @chancez @christarazi @mhofstetter @gentoo-root @pippolo84 @aanm @youngnick @rastislavs @rgo3