Fix golint warnings for daemon/execdriver/* by hqhq · Pull Request #14848 · moby/moby

hqhq · 2015-07-22T09:31:05Z

Addresses: #14756

Signed-off-by: Qiang Huang h.huangqiang@huawei.com

cpuguy83 · 2015-07-22T12:53:47Z

daemon/execdriver/driver.go

Needs more info, i.e. what/where is this callback used?

cpuguy83 · 2015-07-22T13:05:42Z

All in all I think most of the doc strings could use some more detail as things like "Foo stores information about foos" isn't all that helpful.

LK4D4 · 2015-07-22T15:51:10Z

I agree with @cpuguy83

hqhq · 2015-07-23T03:56:43Z

@cpuguy83 @LK4D4 updated comments as you suggested and added some extra comments for some other functions/structs.

Feel free to point them out if you think they need more details, I just thought most of them are quite self explained, which is why they have no docs in the first place.

LK4D4 · 2015-07-23T16:55:54Z

daemon/execdriver/lxc/driver.go

launch :)

mmm, lxc commands. omnomnomnom

cpuguy83 · 2015-07-24T15:28:46Z

daemon/execdriver/utils.go

Same thing as below regarding caps

cpuguy83 · 2015-07-24T15:33:49Z

@hqhq Great work on this! This is a fantastic start.

I did notice we have slightly different doc strings between native/lxc on stuff that should basically be the same as they are just implementing some interface.
I wonder if it's good enough to put the doc details on the Driver methods in the interface definition and just mention in the actual drivers this implements the exec driver <foo> interface, and any pertinent details required for that particular implementation.

hqhq · 2015-07-25T03:08:35Z

@cpuguy83 Updated, now all common docs are in interface, it's easier to maintain, what do you think now?

cpuguy83 · 2015-07-25T03:22:42Z

LGTM

vdemeester · 2015-07-26T20:04:32Z

daemon/execdriver/driver.go

s/do/does/ or s/do/to do/ ?

vdemeester · 2015-07-26T20:15:05Z

@hqhq Could you rebase it so you can add the daemon/execdriver package and sub-packages to hack/make/validate-lint 😉 .

hqhq · 2015-07-27T07:24:53Z

@vdemeester Thanks, updated.

vdemeester · 2015-07-27T07:29:41Z

ah, driver_unsupported*.go needs some lint too 😊.

hqhq · 2015-07-27T08:12:27Z

Fixed, should'v used golint xxx/*.go 😄

jessfraz · 2015-07-28T00:26:26Z

needs rebase sorry

Addresses: moby#14756 Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

hqhq · 2015-07-28T01:40:44Z

@jfrazelle done.

RichardScothern · 2015-07-28T20:33:54Z

LGTM

LK4D4 · 2015-07-28T21:23:20Z

@hqhq Thanks!
LGTM

Fix golint warnings for daemon/execdriver/*

tiborvass · 2015-07-31T00:12:33Z

hack/make/validate-lint

@hqhq you forgot windows! :)

@tiborvass Yeah, I didn't use golint xxx/*.go just goling xxx/ so codes for windows are missed when I sent the PR, windows part is in another PR #15114, PTAL, thanks.

GordonTheTurtle added the status/0-triage label Jul 22, 2015

hqhq force-pushed the hq_golint_execdriver branch 2 times, most recently from 0b4b7c8 to 78b758d Compare July 22, 2015 10:23

runcom added status/2-code-review and removed status/0-triage labels Jul 22, 2015

cpuguy83 reviewed Jul 22, 2015
View reviewed changes

daemon/execdriver/driver.go Outdated

Copy link

Member

cpuguy83 Jul 22, 2015

Choose a reason for hiding this comment

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

Needs more info, i.e. what/where is this callback used?

icecrime mentioned this pull request Jul 22, 2015

Enable golint on the code base #14756

Closed

hqhq force-pushed the hq_golint_execdriver branch from 78b758d to 37c7119 Compare July 23, 2015 03:51

LK4D4 reviewed Jul 23, 2015
View reviewed changes

hqhq force-pushed the hq_golint_execdriver branch from 37c7119 to e2616ff Compare July 24, 2015 07:25

cpuguy83 reviewed Jul 24, 2015
View reviewed changes

daemon/execdriver/utils.go

Copy link

Member

cpuguy83 Jul 24, 2015

Choose a reason for hiding this comment

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

Same thing as below regarding caps

hqhq force-pushed the hq_golint_execdriver branch from e2616ff to 654fa02 Compare July 25, 2015 03:04

vdemeester reviewed Jul 26, 2015
View reviewed changes

daemon/execdriver/driver.go Outdated

Copy link

Member

vdemeester Jul 26, 2015

Choose a reason for hiding this comment

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

s/do/does/ or s/do/to do/ ?

hqhq force-pushed the hq_golint_execdriver branch from 654fa02 to 6b44c35 Compare July 27, 2015 07:24

hqhq force-pushed the hq_golint_execdriver branch from 6b44c35 to dd61f36 Compare July 27, 2015 08:11

hqhq added 2 commits July 28, 2015 08:43

Fix golint warnings for daemon/execdriver/*

3d17c3b

Addresses: moby#14756 Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

Add daemon/execdriver/* to validate-lint

4862d72

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>

hqhq force-pushed the hq_golint_execdriver branch from dd61f36 to 4862d72 Compare July 28, 2015 01:40

LK4D4 added a commit that referenced this pull request Jul 28, 2015

Merge pull request #14848 from hqhq/hq_golint_execdriver

f809037

Fix golint warnings for daemon/execdriver/*

LK4D4 merged commit f809037 into moby:master Jul 28, 2015

hqhq deleted the hq_golint_execdriver branch July 29, 2015 00:40

tiborvass reviewed Jul 31, 2015
View reviewed changes

hqhq mentioned this pull request Aug 3, 2015

Fix golint warnings for daemon/execdriver/windows #15114

Merged

Conversation

hqhq commented Jul 22, 2015

Uh oh!

cpuguy83 Jul 22, 2015

Choose a reason for hiding this comment

Uh oh!

cpuguy83 commented Jul 22, 2015

Uh oh!

LK4D4 commented Jul 22, 2015

Uh oh!

hqhq commented Jul 23, 2015

Uh oh!

LK4D4 Jul 23, 2015

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Jul 23, 2015

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Jul 24, 2015

Choose a reason for hiding this comment

Uh oh!

cpuguy83 commented Jul 24, 2015

Uh oh!

hqhq commented Jul 25, 2015

Uh oh!

cpuguy83 commented Jul 25, 2015

Uh oh!

vdemeester Jul 26, 2015

Choose a reason for hiding this comment

Uh oh!

vdemeester commented Jul 26, 2015

Uh oh!

hqhq commented Jul 27, 2015

Uh oh!

vdemeester commented Jul 27, 2015

Uh oh!

hqhq commented Jul 27, 2015

Uh oh!

jessfraz commented Jul 28, 2015

Uh oh!

hqhq commented Jul 28, 2015

Uh oh!

RichardScothern commented Jul 28, 2015

Uh oh!

LK4D4 commented Jul 28, 2015

Uh oh!

tiborvass Jul 31, 2015

Choose a reason for hiding this comment

Uh oh!

hqhq Jul 31, 2015

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants