Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Fix linter issues #3450

Merged
merged 1 commit into from
Sep 28, 2022
Merged

Fix linter issues #3450

merged 1 commit into from
Sep 28, 2022

Conversation

chandrareddyp
Copy link
Contributor

@chandrareddyp chandrareddyp commented Sep 21, 2022 •
edited
Loading

What this PR does / why we need it

This PR is to fix or skip linter issues for follow directories:
https://github.com/vmware-tanzu/tanzu-framework/tree/main/tkg
https://github.com/vmware-tanzu/tanzu-framework/tree/main/cmd/cli/plugin/#
https://github.com/vmware-tanzu/tanzu-framework/tree/main/cmd/cli/plugin-admin/test

Which issue(s) this PR fixes

Fixes #3437

Describe testing done for PR

  1. Ran the golangci-lint binary locally for /main/tkg directory

Here is the golangci-lint output for /main/tkg directory:

❯ ../hack/tools/bin/golangci-lint run -v --timeout=10m
INFO [config_reader] Config search paths: [./ /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/tkg /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework /Users/cpamuluri/tkg/tasks/plugin899/latest2 /Users/cpamuluri/tkg/tasks/plugin899 /Users/cpamuluri/tkg/tasks /Users/cpamuluri/tkg /Users/cpamuluri /Users /] 
INFO [config_reader] Used config file ../.golangci.yaml 
INFO [lintersdb] Active 32 linters: [bodyclose deadcode depguard dogsled dupl errcheck funlen goconst gocritic gocyclo goheader goimports goprintffuncname gosec gosimple govet ineffassign misspell nakedret noctx nolintlint revive rowserrcheck staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace] 
INFO [loader] Go packages loading at mode 575 (compiled_files|name|types_sizes|deps|exports_file|files|imports) took 6.92946184s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 172.992358ms 
INFO [linters context/goanalysis] analyzers took 37m18.49989688s with top 10 stages: gocritic: 11m2.285754295s, goimports: 5m12.370998612s, buildir: 3m27.822376545s, buildssa: 2m47.777188185s, goheader: 2m11.777559965s, the_only_name: 2m7.551483332s, dupl: 1m30.878916017s, gosec: 59.41655014s, misspell: 41.199387586s, S1038: 40.863702065s 
INFO [runner/skip dirs] Skipped 28 issues from dir test/framework by pattern test/ 
INFO [runner/skip dirs] Skipped 2 issues from dir fakes/providers by pattern fakes/ 
INFO [runner/skip dirs] Skipped 29 issues from dir test/tkgctl/tkgs by pattern test/ 
INFO [runner/skip dirs] Skipped 50 issues from dir test/tkgctl/aws_cc by pattern test/ 
INFO [runner/skip dirs] Skipped 20 issues from dir test/tkgctl/vsphere67 by pattern test/ 
INFO [runner/skip dirs] Skipped 20 issues from dir test/tkgctl/aws by pattern test/ 
INFO [runner/skip dirs] Skipped 22 issues from dir test/tkgctl/azure by pattern test/ 
INFO [runner/skip dirs] Skipped 2 issues from dir test/cmp/strings by pattern test/ 
INFO [runner/skip dirs] Skipped 9 issues from dir test/tkgctl/docker_cc by pattern test/ 
INFO [runner/skip dirs] Skipped 382 issues from dir test/tkgctl/shared by pattern test/ 
INFO [runner/skip dirs] Skipped 30 issues from dir fakes/helper by pattern fakes/ 
INFO [runner/skip dirs] Skipped 52 issues from dir test/tkgctl/tkgs_cc by pattern test/ 
INFO [runner/skip dirs] Skipped 9 issues from dir test/framework/exec by pattern test/ 
INFO [runner/skip dirs] Skipped 22 issues from dir test/tkgctl/docker by pattern test/ 
INFO [runner] Issues before processing: 4720, after processing: 0 
INFO [runner] Processors filtering stat (out/in): skip_files: 4720/4720, exclude-rules: 317/911, cgo: 4720/4720, filename_unadjuster: 4720/4720, path_prettifier: 4720/4720, exclude: 911/1170, skip_dirs: 4043/4720, autogenerated_exclude: 1170/4043, nolint: 0/317, identifier_marker: 1170/1170 
INFO [runner] processing took 502.390842ms with stages: autogenerated_exclude: 236.362159ms, nolint: 152.23899ms, path_prettifier: 40.226779ms, exclude-rules: 31.129274ms, identifier_marker: 29.887926ms, exclude: 7.646704ms, skip_dirs: 2.783195ms, filename_unadjuster: 1.161862ms, cgo: 948.302µs, max_same_issues: 1.367µs, uniq_by_line: 761ns, source_code: 515ns, severity-rules: 471ns, skip_files: 469ns, max_from_linter: 433ns, diff: 392ns, path_shortener: 359ns, max_per_file_from_linter: 346ns, sort_results: 336ns, path_prefixer: 202ns 
INFO [runner] linters took 3m36.773891973s with stages: goanalysis_metalinter: 3m36.271207377s 
INFO File cache stats: 309 entries of total size 2.8MiB 
INFO Memory: 2213 samples, avg is 1624.0MB, max is 2018.4MB 
INFO Execution took 3m43.901736574s
  1. Ran the golangci-lint binary locally for /main/cmd/cli/plugin/# directory
    Here is the golangci-lint output for /main/cmd/cli/plugin/# directory:
❯ /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/hack/tools/bin/golangci-lint run -v --timeout=10m
INFO [config_reader] Config search paths: [./ /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd/cli/plugin/# /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd/cli/plugin /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd/cli /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework /Users/cpamuluri/tkg/tasks/plugin899/latest2 /Users/cpamuluri/tkg/tasks/plugin899 /Users/cpamuluri/tkg/tasks /Users/cpamuluri/tkg /Users/cpamuluri /Users /] 
INFO [config_reader] Used config file ../../../../.golangci.yaml 
INFO [lintersdb] Active 32 linters: [bodyclose deadcode depguard dogsled dupl errcheck funlen goconst gocritic gocyclo goheader goimports goprintffuncname gosec gosimple govet ineffassign misspell nakedret noctx nolintlint revive rowserrcheck staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace] 
INFO [loader] Go packages loading at mode 575 (deps|imports|types_sizes|compiled_files|exports_file|files|name) took 1.176931265s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 1.319415ms 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [runner] Issues before processing: 12, after processing: 0 
INFO [runner] Processors filtering stat (out/in): skip_files: 12/12, identifier_marker: 12/12, exclude: 11/12, nolint: 0/8, cgo: 12/12, exclude-rules: 8/11, autogenerated_exclude: 12/12, skip_dirs: 12/12, filename_unadjuster: 12/12, path_prettifier: 12/12 
INFO [runner] processing took 2.787082ms with stages: nolint: 1.780051ms, exclude-rules: 317.221µs, autogenerated_exclude: 281.343µs, identifier_marker: 234.862µs, path_prettifier: 108.049µs, exclude: 40.36µs, skip_dirs: 15.246µs, filename_unadjuster: 3.073µs, cgo: 1.788µs, max_same_issues: 1.106µs, uniq_by_line: 845ns, max_from_linter: 591ns, max_per_file_from_linter: 569ns, source_code: 380ns, skip_files: 327ns, severity-rules: 294ns, diff: 294ns, path_shortener: 280ns, sort_results: 256ns, path_prefixer: 147ns 
INFO [runner] linters took 673.268071ms with stages: goanalysis_metalinter: 670.364874ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 20 samples, avg is 58.3MB, max is 71.3MB 
INFO Execution took 1.868710187s
  1. Ran the golangci-lint binary locally for /main/cmd/cli/plugin-admin/test directory
    Here is the golangci-lint output for /main/cmd/cli/plugin-admin/test directory:
❯ /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/hack/tools/bin/golangci-lint run -v --timeout=10m
INFO [config_reader] Config search paths: [./ /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd/cli/plugin-admin/test /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd/cli/plugin-admin /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd/cli /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework/cmd /Users/cpamuluri/tkg/tasks/plugin899/latest2/tanzu-framework /Users/cpamuluri/tkg/tasks/plugin899/latest2 /Users/cpamuluri/tkg/tasks/plugin899 /Users/cpamuluri/tkg/tasks /Users/cpamuluri/tkg /Users/cpamuluri /Users /] 
INFO [config_reader] Used config file ../../../../.golangci.yaml 
INFO [lintersdb] Active 32 linters: [bodyclose deadcode depguard dogsled dupl errcheck funlen goconst gocritic gocyclo goheader goimports goprintffuncname gosec gosimple govet ineffassign misspell nakedret noctx nolintlint revive rowserrcheck staticcheck structcheck stylecheck typecheck unconvert unparam unused varcheck whitespace] 
INFO [loader] Go packages loading at mode 575 (imports|compiled_files|exports_file|files|types_sizes|deps|name) took 1.245215018s 
INFO [runner/filename_unadjuster] Pre-built 0 adjustments in 624.669µs 
INFO [linters context/goanalysis] analyzers took 0s with no stages 
INFO [runner] Issues before processing: 4, after processing: 0 
INFO [runner] Processors filtering stat (out/in): skip_files: 4/4, autogenerated_exclude: 4/4, exclude: 4/4, skip_dirs: 4/4, cgo: 4/4, filename_unadjuster: 4/4, path_prettifier: 4/4, identifier_marker: 4/4, nolint: 0/4, exclude-rules: 4/4 
INFO [runner] processing took 1.88986ms with stages: nolint: 798.198µs, autogenerated_exclude: 363.032µs, exclude-rules: 316.92µs, path_prettifier: 197.482µs, identifier_marker: 147.027µs, exclude: 38.64µs, skip_dirs: 21.393µs, cgo: 1.886µs, max_same_issues: 949ns, filename_unadjuster: 877ns, skip_files: 542ns, uniq_by_line: 473ns, source_code: 433ns, max_from_linter: 419ns, severity-rules: 321ns, max_per_file_from_linter: 293ns, diff: 284ns, sort_results: 264ns, path_shortener: 242ns, path_prefixer: 185ns 
INFO [runner] linters took 570.316073ms with stages: goanalysis_metalinter: 568.297184ms 
INFO File cache stats: 0 entries of total size 0B 
INFO Memory: 20 samples, avg is 60.0MB, max is 70.6MB 
INFO Execution took 1.835894484s

Release note

Fixed or skipped linter issues

Additional information

Special notes for your reviewer

Below two GitHub issues created to fix these linter issues permanently
#3478
#3479

@chandrareddyp chandrareddyp requested review from a team as code owners September 21, 2022 21:15
@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/linter-issues-skip1 branch 2 times, most recently from b9eaafe to da3f0bf Compare September 21, 2022 21:27
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220921212726/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220921213635/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220921213725/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@saji-pivotal
Copy link
Contributor

saji-pivotal commented Sep 21, 2022 •
edited
Loading

@chandrareddyp Can you please copy paste the local linter output to the testing section of the PR description to show that the changes in this PR have fixed the linter errors in the modules you're handling?

@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/linter-issues-skip1 branch from 3bff003 to 3be146a Compare September 21, 2022 23:23
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220921233209/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220921233218/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220921233613/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/linter-issues-skip1 branch from 34d6b51 to 3695702 Compare September 21, 2022 23:54
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220922000443/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220922000600/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@vuil
Copy link
Contributor

vuil commented Sep 22, 2022 •
edited
Loading

General comment: please be consistent on the nolint comments, they should not have spaces in the comment, see
https://golangci-lint.run/usage/false-positives/#exclude-issues-by-path

//nolint:xxxx

This was referenced Sep 23, 2022
Open
Open
@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/linter-issues-skip1 branch 2 times, most recently from fabc2ee to 73ce597 Compare September 23, 2022 18:50
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220923185603/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220923190321/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220923190509/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

marckhouzam
marckhouzam reviewed Sep 27, 2022
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

The build seems to fail. If you run make fmt locally it should fix the problem.

@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/linter-issues-skip1 branch 2 times, most recently from 9b1dbae to 149ad5f Compare September 28, 2022 14:43
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220928145335/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@chandrareddyp chandrareddyp force-pushed the topic/cpamuluri/linter-issues-skip1 branch from 3da3257 to 0d9aced Compare September 28, 2022 15:47
@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220928155705/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@github-actions
Copy link

Cluster Generation A/B Results:
https://storage.googleapis.com/tkg-clustergen/3450/20220928155830/clustergen.diff.txt
Author/reviewers:
Please review to verify that the effects on the generated cluster configurations are exactly what the PR intended, and give a thumbs-up if so.

@anujc25 anujc25 added the ok-to-merge PRs should be labelled with this before merging label Sep 28, 2022
marckhouzam
marckhouzam approved these changes Sep 28, 2022
View reviewed changes
Copy link
Contributor

@marckhouzam marckhouzam left a comment

Choose a reason for hiding this comment

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

LGTM

vuil
vuil approved these changes Sep 28, 2022
View reviewed changes
Copy link
Contributor

@vuil vuil left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

@chandrareddyp chandrareddyp merged commit f7ef2a8 into main Sep 28, 2022
@chandrareddyp chandrareddyp deleted the topic/cpamuluri/linter-issues-skip1 branch September 28, 2022 21:13
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Reviewers

@marckhouzam marckhouzam marckhouzam approved these changes

@vuil vuil vuil approved these changes

@anujc25 anujc25 anujc25 approved these changes

@saji-pivotal saji-pivotal saji-pivotal approved these changes

Assignees
No one assigned
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Fix linter issues in modules
6 participants
@chandrareddyp @saji-pivotal @vuil @marckhouzam @anujc25 @vmwclabot