-
Notifications
You must be signed in to change notification settings - Fork 18.8k
Use container status values from api #49874
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after the linting issue is fixed
daemon/wait.go
Outdated
@@ -13,7 +14,7 @@ import ( | |||
// condition is met or if an error occurs waiting for the container (such as a | |||
// context timeout or cancellation). On a successful wait, the exit code of the | |||
// container is returned in the status with a non-nil Err() value. | |||
func (daemon *Daemon) ContainerWait(ctx context.Context, name string, condition container.WaitCondition) (<-chan container.StateStatus, error) { | |||
func (daemon *Daemon) ContainerWait(ctx context.Context, name string, condition containertypes.WaitCondition) (<-chan container.StateStatus, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
daemon/wait.go:17:119: SA1019: container.StateStatus is deprecated: use [container.StateStatus] instead. (staticcheck)
func (daemon *Daemon) ContainerWait(ctx context.Context, name string, condition containertypes.WaitCondition) (<-chan container.StateStatus, error) {
^
func (daemon *Daemon) ContainerWait(ctx context.Context, name string, condition containertypes.WaitCondition) (<-chan container.StateStatus, error) { | |
func (daemon *Daemon) ContainerWait(ctx context.Context, name string, condition containertypes.WaitCondition) (<-chan containertypes.StateStatus, error) { |
But it looks like this is the only remaining use of "github.com/docker/docker/container"
, so we could also remove the alias for the new import instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated it to be consistent with the daemon
package. Another followup change could be done to make them consistent across packages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I later noticed that, so we can keep the alias; linting issue needs to be fixed though to make CI happy 😁
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh! You did, thanks!
Alias and deprecate the status types and constants from the root container package. The root container package is intended for use within the daemon and no the api package. Signed-off-by: Derek McGowan <derek@mcg.dev>
de5cb27
to
1001021
Compare
Alias and deprecate the status types and constants from the root container package. The root container package is intended for use within the daemon and no the api package.
This is a part of separating the dependency of api to non-api packages.
- Human readable description for the release notes