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

[bug] go report tells code needs improvement #124

Closed
allencloud opened this issue Nov 20, 2017 · 5 comments · Fixed by #143
Closed

[bug] go report tells code needs improvement #124

allencloud opened this issue Nov 20, 2017 · 5 comments · Fixed by #143
Assignees
Labels
kind/bug This is bug report for project

Comments

@allencloud
Copy link
Collaborator

Issue Description

In README.md if we click go report, we will turn to link https://goreportcard.com/report/github.com/alibaba/pouch

There we found that there are several parts to be improve, such as:

go vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string.
pouch/cli/pull.go
Line 94: error: unrecognized printf verb 'r' (vet)
Line 102: error: unrecognized printf verb 'r' (vet)
Line 109: error: unrecognized printf verb 'r' (vet)

and

Gocyclo calculates cyclomatic complexities of functions in Go source code. The cyclomatic complexity of a function is calculated according to the following rules: 1 is the base complexity of a function +1 for each 'if', 'for', 'case', '&&' or '||' Go Report Card warns on functions with cyclomatic complexity > 15.
pouch/volume/core.go
Line 95: warning: cyclomatic complexity 16 of function (*Core).CreateVolume() is high (> 15) (gocyclo)
pouch/ctrd/image.go
Line 130: warning: cyclomatic complexity 22 of function (*Client).fetchProgress() is high (> 15) (gocyclo)

and

Misspell Finds commonly misspelled English words
pouch/test/pouch_api_help_test.go
Line 15: warning: "begining" is a misspelling of "beginning" (misspell)
pouch/test/pouch_cli_help_test.go
Line 17: warning: "begining" is a misspelling of "beginning" (misspell)
pouch/test/pouch_cli_version_test.go
Line 17: warning: "begining" is a misspelling of "beginning" (misspell)
pouch/volume/driver/driver.go
Line 150: warning: "registed" is a misspelling of "registered" (misspell)

and so so.

These all needs fixed.

@pouchrobot pouchrobot added the kind/bug This is bug report for project label Nov 20, 2017
@allencloud
Copy link
Collaborator Author

Could you help to solve this for project? @Letty5411

@Letty5411
Copy link
Contributor

Letty5411 commented Nov 20, 2017

@allencloud OK

@allencloud allencloud assigned zhubingbing and unassigned Letty5411 Nov 20, 2017
@allencloud
Copy link
Collaborator Author

I reassigned this issue to @zhubingbing , and maybe bingbing can help. @Letty5411

@zhubingbing
Copy link
Contributor

zhubingbing commented Nov 21, 2017

  1. Issue Description
go vet examines Go source code and reports suspicious constructs, such as Printf calls whose arguments do not align with the format string.
pouch/cli/pull.go
Line 94: error: unrecognized printf verb 'r' (vet)
Line 102: error: unrecognized printf verb 'r' (vet)
Line 109: error: unrecognized printf verb 'r' (vet)

i think fmt.Sprintf formate don’t have “r” parameter, so go vet report this issue.
if an operand implements the Formatter interface, it will be invoked. Formatter provides fine control of formatting.(https://golang.org/pkg/fmt/#Fprintf)
and bar.go(https://github.com/containerd/containerd/blob/master/progress/bar.go#L21) to realize formatter interface. so i thinks there's no problems with this code .

@allencloud
Copy link
Collaborator Author

Actually with my own go vet on my local machine, it will report no error:

pouch (master) $ go vet cli/pull.go
pouch (master) $ go version
go version go1.8.3 darwin/amd64
pouch (master) $

So maybe we can ignore this, and please move on on your work. @zhubingbing
Thanks a lot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug This is bug report for project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants