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

chore(cmd/cli): Reduce cyclomatic complexity of uninstall command #4825

Merged

Conversation

shalier
Copy link
Contributor

@shalier shalier commented Jun 16, 2022

Description:
Separate code into functions to reduce the cyclomatic complexity
Helps unblock #4555

Signed-off-by: Shalier Xia [email protected]

Reduces cyclomatic complexity 30->19
image

image

Testing done:

Affected area:

Functional Area
CLI Tool [x ]
Other [ ]

Please answer the following questions with yes/no.

  1. Does this change contain code from or inspired by another project?

    • Did you notify the maintainers and provide attribution?
  2. Is this a breaking change?

  3. Has documentation corresponding to this change been updated in the osm-docs repo (if applicable)?

@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2022

Codecov Report

Merging #4825 (c05af3e) into main (aa1abf1) will increase coverage by 0.02%.
The diff coverage is 64.40%.

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

@@            Coverage Diff             @@
##             main    #4825      +/-   ##
==========================================
+ Coverage   68.71%   68.73%   +0.02%     
==========================================
  Files         220      220              
  Lines       15907    15927      +20     
==========================================
+ Hits        10930    10947      +17     
- Misses       4925     4928       +3     
  Partials       52       52              
Flag Coverage Δ
unittests 68.73% <64.40%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cmd/cli/uninstall_mesh.go 69.91% <64.40%> (+0.63%) ⬆️
pkg/ticker/ticker.go 83.33% <0.00%> (-3.85%) ⬇️
pkg/certificate/manager.go 77.51% <0.00%> (+0.80%) ⬆️
cmd/cli/util.go 67.04% <0.00%> (+1.13%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aa1abf1...2355600. Read the comment docs.

@shalier shalier marked this pull request as draft June 16, 2022 05:14
@shalier shalier force-pushed the cyclomaticComplexityUninstall branch from 5bdb0b0 to ffc87fa Compare June 19, 2022 22:53
@shalier shalier marked this pull request as ready for review June 20, 2022 08:01
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
@allenlsy
Copy link
Contributor

LGTM other than Thomas's comments

cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
@shalier shalier force-pushed the cyclomaticComplexityUninstall branch from 0fbcb45 to 9ba7ab2 Compare June 20, 2022 20:36
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
@shalier shalier force-pushed the cyclomaticComplexityUninstall branch 4 times, most recently from aaf0a87 to 1141e73 Compare June 28, 2022 19:28
@shalier shalier force-pushed the cyclomaticComplexityUninstall branch from 3272702 to 81f70f8 Compare June 29, 2022 18:22
Copy link
Contributor

@draychev draychev left a comment

Choose a reason for hiding this comment

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

Who can argue against reduction in cyclomatic complexity!

Thank you for these changes @shalier

cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
Copy link
Contributor

@steeling steeling left a comment

Choose a reason for hiding this comment

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

a couple small comments, but otherwise LGTM!

@shalier shalier requested a review from keithmattix as a code owner July 1, 2022 20:30
@shalier shalier requested a review from steeling July 1, 2022 20:31
@shalier shalier force-pushed the cyclomaticComplexityUninstall branch from f994bdb to ef6890d Compare July 1, 2022 20:34
cmd/cli/uninstall_mesh.go Outdated Show resolved Hide resolved
@shalier shalier force-pushed the cyclomaticComplexityUninstall branch from acdb9ec to c05af3e Compare July 7, 2022 16:21
shalier added 6 commits July 7, 2022 09:32
Separate code into functions to reduce the cyclomatic complexity
And adds unit tests to functions created
Helps unblock openservicemesh#4555

Signed-off-by: Shalier Xia <[email protected]>
Signed-off-by: Shalier Xia <[email protected]>
@shalier shalier force-pushed the cyclomaticComplexityUninstall branch from c05af3e to 2355600 Compare July 7, 2022 16:33
@shalier shalier requested a review from jaellio July 8, 2022 16:51
@trstringer trstringer merged commit d5d3a25 into openservicemesh:main Jul 14, 2022
trstringer pushed a commit that referenced this pull request Jul 20, 2022
* chore(cmd/cli): Reduce cyclomatic complexity of uninstall command (#4825)

* chore(cmd/cli): Reduce cyclomatic complexity of uninstall command

Separate code into functions to reduce the cyclomatic complexity
And adds unit tests to functions created
Helps unblock #4555

Signed-off-by: Shalier Xia <[email protected]>
(cherry picked from commit d5d3a25)

* Golang errors pkg (#4904)

This PR partially resolves #4524

* Removes "github.com/pkg/errors" package from the repo
* Wrap errors wherever possible rather than using the string format of the error as part of the message

Signed-off-by: Allen Leigh <[email protected]>
(cherry picked from commit 8030047)

* fix golints and security scan issues (#4915)

fix golints and security for:

1. G112 (ReadHeaderTimeout)
2. prometheus client_go version pinned to bad version

Signed-off-by: Sean Teeling <[email protected]>

(cherry picked from commit f768f64)

* Fix lints by removing superfluous var type from var instantiation (#4917)

Remove unnecessary var types during instantiation to fix lints

Signed-off-by: Sean Teeling <[email protected]>
(cherry picked from commit 9e9f712)

Co-authored-by: Shalier Xia <[email protected]>
Co-authored-by: allenlsy <[email protected]>
Co-authored-by: steeling <[email protected]>
@shalier shalier deleted the cyclomaticComplexityUninstall branch November 16, 2022 00:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants