Skip to content

Fix golint for pkg/mflag#14824

Merged
LK4D4 merged 1 commit intomoby:masterfrom
fcantournet:lint_pkg_mflag
Jul 28, 2015
Merged

Fix golint for pkg/mflag#14824
LK4D4 merged 1 commit intomoby:masterfrom
fcantournet:lint_pkg_mflag

Conversation

@fcantournet
Copy link
Contributor

Addresses (partly) : #14756

Signed-off-by: Félix Cantournet felix.cantournet@cloudwatt.com

Copy link
Member

Choose a reason for hiding this comment

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

better to be fs
you also changed the method receiver name and tests are failing for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok will change it. (and ty silly me)
I'll add pkg/mflag to the automatically linted packges too.

Copy link
Contributor

Choose a reason for hiding this comment

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

there is a compilation issue because cmd is still referenced in line 1077.

@fcantournet fcantournet force-pushed the lint_pkg_mflag branch 2 times, most recently from 3524205 to 2acc149 Compare July 21, 2015 21:53
@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 21, 2015
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 21, 2015
@fcantournet
Copy link
Contributor Author

@runcom does this LGTY ?

@calavera
Copy link
Contributor

Please, do NOT merge this PR just yet. I'd like to land #13771 first.

@calavera
Copy link
Contributor

Nothing bad about the changes, but we need the other PR merged as soon as possible and I'd like to avoid any friction before that happens.

@calavera
Copy link
Contributor

@fcantournet we merged #13771 yesterday and this is ready to merge. Unfortunately, we added a couple new methods to the mflag package in master. Do you want to rebase this change and cover the new additions?

@fcantournet
Copy link
Contributor Author

@calavera Sure, I'll rebase and finish the job.

@calavera
Copy link
Contributor

LGTM

Copy link
Member

Choose a reason for hiding this comment

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

s/Indicates/indicates/ and a period at the end 😊

@fcantournet
Copy link
Contributor Author

@calavera test seem flaky. The errors don't seem to be link with this patch. is there a way to retrigger the checks ?

Signed-off-by: Félix Cantournet <felix.cantournet@cloudwatt.com>
@RichardScothern
Copy link
Contributor

LGTM

1 similar comment
@dmcgowan
Copy link
Member

LGTM

@LK4D4
Copy link
Contributor

LK4D4 commented Jul 28, 2015

Ok, it compiles
LGTM

LK4D4 added a commit that referenced this pull request Jul 28, 2015
@LK4D4 LK4D4 merged commit 2c16229 into moby:master Jul 28, 2015
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.

8 participants