lint: opts, trust by sevki · Pull Request #14813 · moby/moby

sevki · 2015-07-21T16:17:56Z

contributes to #14756
for opts

opts/ip.go:9:6: type IpOpt should be IPOpt
opts/ip.go:13:1: exported function NewIpOpt should have comment or be unexported
opts/ip.go:13:6: func NewIpOpt should be NewIPOpt
opts/ip.go:21:1: exported method IpOpt.Set should have comment or be unexported
opts/ip_test.go:48:2: var invalidIp should be invalidIP
opts/ulimit.go:9:6: exported type UlimitOpt should have comment or be unexported
opts/ulimit.go:13:1: exported function NewUlimitOpt should have comment or be unexported
opts/ulimit.go:17:1: exported method UlimitOpt.Set should have comment or be unexported
opts/ulimit.go:37:1: exported method UlimitOpt.GetList should have comment or be unexported

for trust

trust/service.go:11:6: exported type NotVerifiedError should have comment or be unexported
trust/service.go:17:1: exported method TrustStore.CheckKey should have comment or be unexported
trust/service.go:51:1: exported method TrustStore.UpdateBase should have comment or be unexported
trust/trusts.go:19:6: exported type TrustStore should have comment or be unexported
trust/trusts.go:19:6: type name will be used as trust.TrustStore by other packages, and that stutters; consider calling this Store
trust/trusts.go:40:1: exported function NewTrustStore should have comment or be unexported

runcom · 2015-07-21T17:39:59Z

@sevki there are errors in unit tests

sevki · 2015-07-21T18:03:47Z

@runcom yup, just figured out make doesn't run tests, have to do make test before submitting 😦

vdemeester · 2015-07-22T13:22:18Z

@sevki I tend to run make validate test-unit each time 😉.

I think it would be safer (and clearer) to stash your commits 😉.

calavera · 2015-07-22T18:57:31Z

LGTM

runcom · 2015-07-23T16:45:16Z

opts/ip.go

s/a IP/an IP/ and line should end with a dot

runcom · 2015-07-23T17:00:01Z

@sevki thanks! Can you also squash your commits to just 2 (one for opts and one for trust)?

vdemeester · 2015-07-25T11:47:36Z

trust/service.go

This comments doesn't really say anything.
"NotVerifiedError error is returned when …" (I'm still thinking what to put here 😅 )

vdemeester · 2015-07-26T19:52:50Z

@sevki you're gonna need a rebase too 😉 and add the package trust and opts in ./hack/make/validate-lint too 😊.

RichardScothern · 2015-07-28T18:09:35Z

opts/ulimit.go

Ulmits -> Ulimits

dmcgowan · 2015-07-28T20:58:03Z

Looks like an update was pushed but another rebase is still needed, also need to address the comment about ./hack/make/validate-lint from @vdemeester

tiborvass · 2015-07-29T15:33:10Z

Sorry @sevki needs a rebase :(

sevki · 2015-07-29T15:36:15Z

@tiborvass merge all commits?

tiborvass · 2015-07-29T15:38:00Z

@sevki sure why not, but either way you need to rebase on master :)

sevki · 2015-07-29T15:43:42Z

ok.
On Wed, 29 Jul 2015 at 18:39 Tibor Vass notifications@github.com wrote:

@sevki https://github.com/sevki sure why not, but either way you need
to rebase on master :)

—
Reply to this email directly or view it on GitHub
#14813 (comment).

gdevillele · 2015-07-29T21:45:02Z

opts/ulimit.go

add period at the end of func comments

gdevillele · 2015-07-29T21:45:09Z

opts/ulimit.go

add period at the end of func comments

tiborvass · 2015-07-29T22:13:01Z

@sevki sorry needs another rebase

tiborvass · 2015-07-30T19:59:08Z

Ping @sevki

sevki · 2015-07-30T21:23:16Z

yes?
On Thu, 30 Jul 2015 at 23:00 Tibor Vass notifications@github.com wrote:

Ping @sevki https://github.com/sevki

—
Reply to this email directly or view it on GitHub
#14813 (comment).

tiborvass · 2015-07-30T21:24:19Z

@sevki would you mind rebasing please? :)

Signed-off-by: Sevki Hasirci <s@sevki.org>

icecrime · 2015-08-05T02:33:15Z

It seems it doesn't build :-(

vdemeester · 2015-08-05T06:46:17Z

@sevki I think you had a problem while rebasing maybe ? (because there is some merge commit and thus non-signed one). If you need help, feel free to ping us on #docker / #docker-dev irc channel 😉

contributes to moby#14756 Signed-off-by: Sevki Hasirci <s@sevki.org>

sevki · 2015-08-11T14:30:30Z

@vdemeester done

vdemeester · 2015-08-13T07:45:22Z

@sevki There is still two commit so it's not rebased 😅. You should do something like that : http://stackoverflow.com/a/2568581/89249.

git rebase --interactive HEAD~2
# […]

sevki · 2015-08-13T12:53:41Z

yeah two different folders two different commits
On Thu, 13 Aug 2015 at 10:46 Vincent Demeester notifications@github.com
wrote:

@sevki https://github.com/sevki There is still two commit so it's not
rebased [image: 😅]. You should do something like that :
http://stackoverflow.com/a/2568581/89249.

git rebase --interactive HEAD~2# […]

—
Reply to this email directly or view it on GitHub
#14813 (comment).

vdemeester · 2015-08-13T13:03:00Z

@sevki oh my bad >.<

vdemeester · 2015-08-23T19:57:16Z

@sevki Could you take a look to the 2 left comments ? Then I think we'll be good to go 😉

LK4D4 · 2015-08-26T21:42:36Z

@vdemeester Do you want to carry this PR?

vdemeester · 2015-08-26T21:59:11Z

@LK4D4 I could (tomorrow morning though :-P )

Carry #14813 on linting package opts and trust

vdemeester · 2015-08-27T20:00:03Z

@sevki Thanks for the hard work, it's been carried and merged 😉 🎉

sevki · 2015-08-28T01:46:03Z

@vdemeester cheers.

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

icecrime mentioned this pull request Jul 21, 2015

Enable golint on the code base #14756

Closed

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 21, 2015

sevki force-pushed the 14756-opts branch from 89fd479 to 6dbbe7d Compare July 21, 2015 18:05

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 21, 2015

sevki changed the title ~~Opts lint issues, ip and ulimit~~ lint: opts, trust Jul 21, 2015

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

runcom reviewed Jul 23, 2015
View reviewed changes

opts/ip.go Outdated

Copy link

Member

runcom Jul 23, 2015

Choose a reason for hiding this comment

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

s/a IP/an IP/ and line should end with a dot

vdemeester reviewed Jul 25, 2015
View reviewed changes

sevki force-pushed the 14756-opts branch from 9fee31a to 68e125d Compare July 28, 2015 17:53

RichardScothern reviewed Jul 28, 2015
View reviewed changes

opts/ulimit.go Outdated

Copy link

Contributor

RichardScothern Jul 28, 2015

Choose a reason for hiding this comment

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

Ulmits -> Ulimits

sevki force-pushed the 14756-opts branch from 68e125d to 7a4a379 Compare July 28, 2015 18:19

gdevillele reviewed Jul 29, 2015
View reviewed changes

opts/ulimit.go Outdated

Copy link

Contributor

gdevillele Jul 29, 2015

Choose a reason for hiding this comment

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

add period at the end of func comments

gdevillele reviewed Jul 29, 2015
View reviewed changes

opts/ulimit.go Outdated

Copy link

Contributor

gdevillele Jul 29, 2015

Choose a reason for hiding this comment

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

add period at the end of func comments

GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Aug 4, 2015

sevki force-pushed the 14756-opts branch from 0ac827c to a0f82f8 Compare August 4, 2015 10:00

Opts lint issues, ip and ulimit

8026bf4

Signed-off-by: Sevki Hasirci <s@sevki.org>

sevki force-pushed the 14756-opts branch from a0f82f8 to 5328446 Compare August 4, 2015 10:36

GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Aug 4, 2015

golint: trust

d546b24

contributes to moby#14756 Signed-off-by: Sevki Hasirci <s@sevki.org>

sevki force-pushed the 14756-opts branch from 5328446 to d546b24 Compare August 5, 2015 16:07

vdemeester mentioned this pull request Aug 27, 2015

Carry #14813 on linting package opts and trust #15882

Merged

cpuguy83 closed this in #15882 Aug 27, 2015

cpuguy83 added a commit that referenced this pull request Aug 27, 2015

Merge pull request #15882 from vdemeester/14756-lint-opts-trust

ba019dc

Carry #14813 on linting package opts and trust

Conversation

sevki commented Jul 21, 2015

Uh oh!

runcom commented Jul 21, 2015

Uh oh!

sevki commented Jul 21, 2015

Uh oh!

vdemeester commented Jul 22, 2015

Uh oh!

calavera commented Jul 22, 2015

Uh oh!

runcom Jul 23, 2015

Choose a reason for hiding this comment

Uh oh!

runcom commented Jul 23, 2015

Uh oh!

vdemeester Jul 25, 2015

Choose a reason for hiding this comment

Uh oh!

vdemeester commented Jul 26, 2015

Uh oh!

RichardScothern Jul 28, 2015

Choose a reason for hiding this comment

Uh oh!

dmcgowan commented Jul 28, 2015

Uh oh!

tiborvass commented Jul 29, 2015

Uh oh!

sevki commented Jul 29, 2015

Uh oh!

tiborvass commented Jul 29, 2015

Uh oh!

sevki commented Jul 29, 2015

Uh oh!

gdevillele Jul 29, 2015

Choose a reason for hiding this comment

Uh oh!

gdevillele Jul 29, 2015

Choose a reason for hiding this comment

Uh oh!

tiborvass commented Jul 29, 2015

Uh oh!

tiborvass commented Jul 30, 2015

Uh oh!

sevki commented Jul 30, 2015

Uh oh!

tiborvass commented Jul 30, 2015

Uh oh!

icecrime commented Aug 5, 2015

Uh oh!

vdemeester commented Aug 5, 2015

Uh oh!

sevki commented Aug 11, 2015

Uh oh!

vdemeester commented Aug 13, 2015

Uh oh!

sevki commented Aug 13, 2015

Uh oh!

vdemeester commented Aug 13, 2015

Uh oh!

vdemeester commented Aug 23, 2015

Uh oh!

LK4D4 commented Aug 26, 2015

Uh oh!

vdemeester commented Aug 26, 2015

Uh oh!

vdemeester commented Aug 27, 2015

Uh oh!

sevki commented Aug 28, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

12 participants