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

Fix simple lint issues under addons and plugin/package directories #3516

Merged
merged 1 commit into from
Oct 4, 2022

Conversation

adduarte
Copy link

@adduarte adduarte commented Sep 30, 2022

What this PR does / why we need it

Files under the addons directory and cmd/cli/plugin/package directory are failing some golangci-lint tests. This pr addresses some of those issues

Which issue(s) this PR fixes

Fixes # 3436

Describe testing done for PR

584a33a5 (HEAD -> topic/adduarte/fix_addons_lint_error, origin/topic/adduarte/fix_addons_lint_error) Fix various lint error
aa666877 (origin/main) Add doc with details on BoM usage (#3491)
aduarte@aduarte-a01 tanzu-framework % cd addons
aduarte@aduarte-a01 addons % ../hack/tools/bin/golangci-lint run
aduarte@aduarte-a01 addons % cd ..
aduarte@aduarte-a01 tanzu-framework % cd cmd/cli/plugin/package 
aduarte@aduarte-a01 package % ../../../../hack/tools/bin/golangci-lint run
aduarte@aduarte-a01 package % 

Release note

Fix lint issues under addons/
Fix lint issues under cmd/cli/plugin/packages

Additional information

Special notes for your reviewer

Copy link
Contributor

@vijaykatam vijaykatam left a comment

Choose a reason for hiding this comment

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

Fixes look legit to me. Update PR title and should be good.

@codecov
Copy link

codecov bot commented Sep 30, 2022

Codecov Report

Merging #3516 (a4e3285) into main (74a09a7) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head a4e3285 differs from pull request most recent head 584a33a. Consider uploading reports for the commit 584a33a to get more accurate results

@@            Coverage Diff             @@
##             main    #3516      +/-   ##
==========================================
- Coverage   45.55%   45.55%   -0.01%     
==========================================
  Files         360      360              
  Lines       38870    38869       -1     
==========================================
- Hits        17706    17705       -1     
  Misses      19560    19560              
  Partials     1604     1604              
Impacted Files Coverage Δ
...ons/controllers/packageinstallstatus_controller.go 77.99% <ø> (-1.16%) ⬇️
addons/pkg/util/gvr_helper.go 69.09% <ø> (ø)
addons/webhooks/clusterbootstrap_webhook.go 24.03% <ø> (ø)
addons/controllers/clusterbootstrap_controller.go 64.86% <100.00%> (-0.03%) ⬇️
...til/clusterbootstrapclone/clusterbootstrapclone.go 70.37% <100.00%> (ø)
addons/webhooks/cluster_pause_webhook.go 80.00% <100.00%> (ø)
addons/controllers/machine_controller.go 68.68% <0.00%> (+3.03%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@adduarte adduarte changed the title Topic/adduarte/fix addons lint error Fix simple lint issues in under addons directory Oct 3, 2022
@adduarte adduarte added the ok-to-merge PRs should be labelled with this before merging label Oct 3, 2022
@adduarte adduarte changed the title Fix simple lint issues in under addons directory Fix simple lint issues under addons and plugin/package directories Oct 3, 2022
Remove whitespaces
Removed duplicate imports
Replaced duplicate strings with const (goconst)
Removed deadcode
Added missing headers (goheader)
Removed shadow declarations (govet)
Added package comment (stylecheck)
Removed unused code (unused)
Removed unused value from for loop (revive)
Simplified function signature (gocritic)
Removed unecessary nil check for map (gosimple)
Removed possible security issue
Importing "_ net/http/pprof" could lead to security problems.
  error is: G108: Profiling endpoint is automatically exposed on /debug/pprof (gosec)
  see https://docs.embold.io/gosec-high-level-issues/ for more details
Added //nolint to correct cyclomatic complexity  and funclen
 The functions being tagged with //nolint have already been in production
 for some time. Correcting the lint error requires heavy refactoring
 which could introduce problems or new bugs. At this time is best to
 flagg the functions with //nolint because the risk/reward ratio of refactoring
 is too high. There is no reward at this time since this functions have already
 gone extensive testing elsewhere.
@adduarte adduarte force-pushed the topic/adduarte/fix_addons_lint_error branch from a4e3285 to 584a33a Compare October 3, 2022 21:58
@adduarte adduarte merged commit 34d7617 into main Oct 4, 2022
@adduarte adduarte deleted the topic/adduarte/fix_addons_lint_error branch October 4, 2022 21:18
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
cla-not-required ok-to-merge PRs should be labelled with this before merging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants