Skip to content

fix golint errors/warnings#14785

Merged
tiborvass merged 1 commit intomoby:masterfrom
brahmaroutu:lint_api_server
Jul 29, 2015
Merged

fix golint errors/warnings#14785
tiborvass merged 1 commit intomoby:masterfrom
brahmaroutu:lint_api_server

Conversation

@brahmaroutu
Copy link
Contributor

Addresses #14756
Signed-off-by: Srini Brahmaroutu srbrahma@us.ibm.com

Copy link
Member

Choose a reason for hiding this comment

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

@brahmaroutu you don't need to add the -. I think the "look" of godoc will be a little weird.

Copy link
Member

Choose a reason for hiding this comment

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

agree with @vdemeester

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this change, as APIServerConfig repeat the api/server part. Config would be simpler 😊. Below you end up with apiserver.APIServerConfig in daemon.go which would look way cleaner like apiserver.Config.

@vdemeester
Copy link
Member

Few comments. See also #14751 (comment), you need a period at the end of the sentences for the func comments. 😉

@brahmaroutu
Copy link
Contributor Author

@vdemeester Please review.

Copy link
Member

Choose a reason for hiding this comment

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

s/allow/allows/

@vdemeester
Copy link
Member

@brahmaroutu Could you add the package api/server to ./hack/make/validate-lint.

@brahmaroutu brahmaroutu force-pushed the lint_api_server branch 4 times, most recently from 9b1efc9 to 056ef00 Compare July 27, 2015 14:00
Copy link
Contributor

Choose a reason for hiding this comment

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

let's write http.Server and net.Listener to have reference to real types in godoc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants