Refactor to new engine-api events api #25853
Conversation
e8ae895 to
997b3f3
Compare
|
ping 👼 |
api/client/container/run.go
Outdated
There was a problem hiding this comment.
"Error waiting for..."?
Also, error string should generally start with a lowercase character.
There was a problem hiding this comment.
Could probably just use errors.Wrap here, as well.
|
Design LGTM |
157f687 to
feab8d6
Compare
|
we are we on this? tests are failing though. |
|
I'll take a look at the tests. |
feab8d6 to
3d9848b
Compare
cli/command/container/run.go
Outdated
There was a problem hiding this comment.
I believe this should use the existing ctx, and not create a new one. Unless there is some special reason this needs its own context? If that's the case I think that warrants a comment about it.
There was a problem hiding this comment.
👍 i noticed that also while fixing the test. I think this was due to a recent rebase where the context was added. Don't remember one when i first wrote this. 👼
3d9848b to
4e4ac03
Compare
|
ping @jhorwit2 👼 |
|
@vdemeester @jhorwit2 Let's try to get this merged. This is a net positive change. |
|
@stevvooe @vdemeester sorry for delay. I fixed one of the issues (was with |
962465b to
969657c
Compare
Signed-off-by: Josh Horwitz <horwitzja@gmail.com>
969657c to
d6bd79c
Compare
|
@vdemeester should be all good now! 👼 |
|
LGTM |
Refactor to new engine-api events api
Refactor to new engine-api events api
ref docker-archive-public/docker.engine-api#315
- What I did
refactored to the new engine-api proposed events api.
- How I did it
Found all occurrences of the old Events call and replaced it with the new one.
- How to verify it
Run unit tests and integration tests.
- Description for the changelog
Refactored events api to updated engine-api interface
Signed-off-by: Josh Horwitz horwitzja@gmail.com
cc @vdemeester