Skip to content

Conversation

@runcom
Copy link
Member

@runcom runcom commented Apr 12, 2015

Signed-off-by: Antonio Murdaca me@runcom.ninja

@LK4D4
Copy link
Contributor

LK4D4 commented Apr 13, 2015

@runcom Can you split your changes to two commits, pls?

@runcom
Copy link
Member Author

runcom commented Apr 13, 2015

Sure!

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe if err != nil?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, if err is nil it means api call was ok, i'm looking for failure here

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops~, i misunderstand that, ignore my noise

Signed-off-by: Antonio Murdaca <me@runcom.ninja>
@runcom runcom force-pushed the remove-job-container-stats branch 2 times, most recently from 9846255 to 65a0563 Compare April 13, 2015 06:40
Copy link
Member

Choose a reason for hiding this comment

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

Creating a flusher here but daemon.ContainerStats only accepts an io.Writer?

Copy link
Member Author

Choose a reason for hiding this comment

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

Missed that

Copy link
Contributor

Choose a reason for hiding this comment

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

it should be io.Writer. Flush called automatically on each write.

@LK4D4
Copy link
Contributor

LK4D4 commented Apr 13, 2015

LGTM

1 similar comment
@cpuguy83
Copy link
Member

LGTM

cpuguy83 added a commit that referenced this pull request Apr 13, 2015
@cpuguy83 cpuguy83 merged commit 12eff0c into moby:master Apr 13, 2015
@runcom runcom deleted the remove-job-container-stats branch May 23, 2015 10:55
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.

5 participants