Skip to content

image fix lint errors/warnings#14786

Merged
LK4D4 merged 1 commit intomoby:masterfrom
brahmaroutu:lint_image
Jul 27, 2015
Merged

image fix lint errors/warnings#14786
LK4D4 merged 1 commit intomoby:masterfrom
brahmaroutu:lint_image

Conversation

@brahmaroutu
Copy link
Copy Markdown
Contributor

@brahmaroutu brahmaroutu commented Jul 21, 2015

Addresses #14756
Signed-off-by: Srini Brahmaroutu [email protected]

@GordonTheTurtle GordonTheTurtle added the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 21, 2015
cpuguy83
cpuguy83 reviewed Jul 21, 2015
View reviewed changes
Comment thread image/image.go Outdated
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 Jul 21, 2015

Choose a reason for hiding this comment

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

Images stores the image configuration

@icecrime icecrime mentioned this pull request Jul 21, 2015
Closed
@cpuguy83
Copy link
Copy Markdown
Member

cpuguy83 commented Jul 21, 2015

LGTM

@runcom
Copy link
Copy Markdown
Member

runcom commented Jul 21, 2015

@brahmaroutu seems like you're committing as root and it's not signed could you check pls?

LGTM though

stevvooe
stevvooe reviewed Jul 21, 2015
View reviewed changes
Comment thread image/image.go Outdated
Copy link
Copy Markdown
Contributor

@stevvooe stevvooe Jul 21, 2015

Choose a reason for hiding this comment

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

Again, please document the usage of the type. The goal is not to have golint pass but to create viable documentation.

@brahmaroutu brahmaroutu force-pushed the lint_image branch 2 times, most recently from 0c2aa21 to ec83c21 Compare July 22, 2015 04:57
@GordonTheTurtle GordonTheTurtle removed the dco/no Automatically set by a bot when one of the commits lacks proper signature label Jul 22, 2015
vdemeester
vdemeester reviewed Jul 22, 2015
View reviewed changes
Comment thread image/image.go Outdated
Copy link
Copy Markdown
Member

@vdemeester vdemeester Jul 22, 2015

Choose a reason for hiding this comment

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

Should the field be here or inline (in the struct Image).

Copy link
Copy Markdown
Contributor Author

@brahmaroutu brahmaroutu Jul 22, 2015

Choose a reason for hiding this comment

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

I moved the comments into the structure. The doc will be slightly different, comments will be printed with the struct and then the comment about struct.

@vdemeester
Copy link
Copy Markdown
Member

vdemeester commented Jul 25, 2015

@brahmaroutu could you add a period at the end of the doc sentences ? :blush:

@vdemeester
Copy link
Copy Markdown
Member

vdemeester commented Jul 26, 2015

@brahmaroutu could you also add the package image to ./hack/make/validate-lint.

@brahmaroutu
/image - fix lint errors/warnings
b061572 
Addresses moby#14756
Signed-off-by: Srini Brahmaroutu <[email protected]>
@LK4D4
Copy link
Copy Markdown
Contributor

LK4D4 commented Jul 27, 2015

Cool, LGTM

LK4D4 added a commit that referenced this pull request Jul 27, 2015
@LK4D4
Merge pull request #14786 from brahmaroutu/lint_image
ae5c8e3 
image fix lint errors/warnings
@LK4D4 LK4D4 merged commit ae5c8e3 into moby:master Jul 27, 2015
@brahmaroutu brahmaroutu deleted the lint_image branch August 12, 2015 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

status/2-code-review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@brahmaroutu @cpuguy83 @runcom @vdemeester @LK4D4 @stevvooe @GordonTheTurtle