golint fixes on runconfig by vdemeester · Pull Request #14751 · moby/moby

vdemeester · 2015-07-20T09:45:29Z

Had a 3 hours long train trip so I did a golint run on the runconfig package. Mostly documentation, even on Config and HostConfig 🐸.

If it feels unecessary feel free to close the PR. 🐧

Signed-off-by: Vincent Demeester vincent@sbr.pm

runcom · 2015-07-20T10:08:14Z

Amazing, LGTM if janky's happy

cpuguy83 · 2015-07-20T14:00:51Z

runconfig/hostconfig.go

I wouldn't say lxc here, unless I'm mistaken this is specifically about caps available to a user in a container, not really lxc specific.

Oh you're right, typo here, will fix 😉

Hum on this one the structure is LxcConfig, but yeah, the comment on the CapList were wrong, I've updated it.

cpuguy83 · 2015-07-20T14:02:24Z

Just a couple of nits regarding the mentioning specifically of lxc. in CapList

LK4D4 · 2015-07-20T15:48:30Z

runconfig/config.go

I think it worth to mention that this method is needed to implement json.Marshaller. And I think this is main usage of this method.

s/Marshal (or serialize)/marshals (or serializes)/

runcom · 2015-07-20T22:36:03Z

@vdemeester there are some compiling errors

vdemeester · 2015-07-21T04:55:44Z

@runcom oups.. forgot to add a change before commit/push..

vdemeester · 2015-07-21T06:24:11Z

runconfig/hostconfig.go

Wasn't sure about adding the json:… notation here, but as it is in the API and consumed/produced already by other external components, I though it would be best to make it so the json in the API doesn't change (to not break external tools).

Sounds safest to me.

duglin · 2015-07-21T16:19:57Z

runconfig/hostconfig.go

s/unmarshal (or deserialize)/unmarshals (or deserializes)/

duglin · 2015-07-21T16:29:13Z

@vdemeester minor comments. But per @tiborvass's irc comment, you need a period at the end of the sentences for the func comments.
https://botbot.me/freenode/docker-dev/2015-07-21/?msg=45149757&page=4

vdemeester · 2015-07-22T07:13:53Z

@duglin thx 😊. Updated it, I think I've added a period everywhere 😝.

LK4D4 · 2015-07-22T15:30:55Z

@vdemeester Need rebase :)

vdemeester · 2015-07-22T15:43:22Z

@LK4D4 oh, already ? 😊 Done 😉

vdemeester · 2015-07-25T09:14:39Z

Rebased again and added godoc on *_windows.go files 😝

jessfraz · 2015-07-27T17:37:09Z

ah man needs rebase, then ping me ;)

Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester · 2015-07-27T21:33:42Z

@jfrazelle yep, systemd/booted.go removed, that's why 👯.
Rebased, weird errors on janky/experimental few hours ago, seems to be going smoothly this time 😅.

jessfraz · 2015-07-27T22:01:06Z

LGTM

golint fixes on runconfig

vdemeester · 2015-07-27T22:02:06Z

yay 🎉

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

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

cpuguy83 reviewed Jul 20, 2015
View reviewed changes

vdemeester force-pushed the runconfig-lint branch from 335f39b to 09067b2 Compare July 20, 2015 14:40

LK4D4 reviewed Jul 20, 2015
View reviewed changes

icecrime mentioned this pull request Jul 20, 2015

Enable golint on the code base #14756

Closed

vdemeester force-pushed the runconfig-lint branch 2 times, most recently from 60cbac5 to eec56bb Compare July 20, 2015 21:34

vdemeester force-pushed the runconfig-lint branch 2 times, most recently from 778f4ad to 843fb03 Compare July 21, 2015 06:16

vdemeester reviewed Jul 21, 2015
View reviewed changes

duglin reviewed Jul 21, 2015
View reviewed changes

runconfig/hostconfig.go Outdated

Copy link

Contributor

duglin Jul 21, 2015

Choose a reason for hiding this comment

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

s/unmarshal (or deserialize)/unmarshals (or deserializes)/

vdemeester force-pushed the runconfig-lint branch 2 times, most recently from 52bbfa5 to 6cf9c82 Compare July 22, 2015 07:12

vdemeester force-pushed the runconfig-lint branch from 6cf9c82 to 0b0be08 Compare July 22, 2015 12:16

vdemeester force-pushed the runconfig-lint branch from 0b0be08 to 5f76e4d Compare July 22, 2015 15:42

vdemeester force-pushed the runconfig-lint branch from 5f76e4d to 884c00a Compare July 22, 2015 16:10

vdemeester mentioned this pull request Jul 25, 2015

fix golint errors/warnings #14785

Merged

vdemeester force-pushed the runconfig-lint branch from 884c00a to 0acd1c1 Compare July 25, 2015 09:12

vdemeester force-pushed the runconfig-lint branch from 0acd1c1 to c7427cb Compare July 26, 2015 16:43

Lint fixes on runconfig

5170a2c

Signed-off-by: Vincent Demeester <vincent@sbr.pm>

vdemeester force-pushed the runconfig-lint branch from c7427cb to 5170a2c Compare July 27, 2015 19:26

jessfraz pushed a commit that referenced this pull request Jul 27, 2015

Merge pull request #14751 from vdemeester/runconfig-lint

542de78

golint fixes on runconfig

jessfraz merged commit 542de78 into moby:master Jul 27, 2015

vdemeester deleted the runconfig-lint branch July 27, 2015 22:01

Conversation

vdemeester commented Jul 20, 2015

Uh oh!

runcom commented Jul 20, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cpuguy83 commented Jul 20, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

runcom commented Jul 20, 2015

Uh oh!

vdemeester commented Jul 21, 2015

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

duglin commented Jul 21, 2015

Uh oh!

vdemeester commented Jul 22, 2015

Uh oh!

LK4D4 commented Jul 22, 2015

Uh oh!

vdemeester commented Jul 22, 2015

Uh oh!

vdemeester commented Jul 25, 2015

Uh oh!

jessfraz commented Jul 27, 2015

Uh oh!

vdemeester commented Jul 27, 2015

Uh oh!

jessfraz commented Jul 27, 2015

Uh oh!

vdemeester commented Jul 27, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants