api/types: move various types to api/types/(images|containers|swarm) #46483
api/types: move various types to api/types/(images|containers|swarm) #46483thaJeztah merged 10 commits intomoby:masterfrom
Conversation
|
Looks like I never pushed this one (I have another one pending, but that one still needed some work around search and auth to prevent a circular import) |
f572afd to
415e1d1
Compare
cd3365f to
231484b
Compare
82d806a to
79d1f77
Compare
There was a problem hiding this comment.
LGTM; although it's probably worth noting that we are starting to have too many image(s) modules 😄
"github.com/containerd/containerd/images"
"github.com/docker/docker/image"
"github.com/docker/docker/daemon/images"
imagetypes "github.com/docker/docker/api/types/image"
OHMAN, yes, we do, and with the containerd-image-store integration that definitely became more apparent. We may be able to avoid some of these if we work on cleaner separation of API types and internal types, but I guess it's something we will have to deal with for some time. Perhaps we should enforce aliases for some of these; same as we did for the containerd-errdefs package; moby/hack/validate/golangci-lint.yml Lines 33 to 36 in aacd100 |
|
oh! needs another rebase (due to the containerd logs changes) |
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
79d1f77 to
ebef4ef
Compare
| GetNode(string) (types.Node, error) | ||
| UpdateNode(string, uint64, types.NodeSpec) error | ||
| ServiceLogs(context.Context, *backend.LogSelector, *types.ContainerLogsOptions) (<-chan *backend.LogMessage, error) | ||
| GetNodes(types.NodeListOptions) ([]swarm.Node, error) |
There was a problem hiding this comment.
Maybe it's in another commit, shouldn't types.NodeListOptions also go to api/types/Swarm?
There was a problem hiding this comment.
Think I didn't get to those (nodes, tasks) yet. I think I was still somewhat looking if everything should be swarm or if we should consider some of those to be separate entities (service, node, task), but doing so may require some additional thinking.
rumpl
left a comment
There was a problem hiding this comment.
I have one question but LGTM otherwise
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)