Add interfacer and unconvert linters#34625
Conversation
|
The 2 places where |
|
Yes, I considered making that change, but I decided against it (at least for now). There are cases (like these) where I think it's actually better to accept a more specific type, even if it's only being used as a string, because it gives the caller more information about the supported values.
|
Signed-off-by: Daniel Nephin <dnephin@docker.com>
Signed-off-by: Daniel Nephin <dnephin@docker.com>
| ContainerExecInspect(id string) (*backend.ExecInspect, error) | ||
| ContainerExecResize(name string, height, width int) error | ||
| ContainerExecStart(ctx context.Context, name string, stdin io.ReadCloser, stdout io.Writer, stderr io.Writer) error | ||
| ContainerExecStart(ctx context.Context, name string, stdin io.Reader, stdout io.Writer, stderr io.Writer) error |
There was a problem hiding this comment.
This one seems a bit strange.
I'd expect to need to close stdin if the exec/container process exits.
There was a problem hiding this comment.
Yes, that's what I was thinking too, but apparently that's not the case.
I'll take another look to find out why.
There was a problem hiding this comment.
It looks like the caller is responsible for closing the the reader, which is why this isn't using ReadCloser
|
ping. Anything I should change here, or do my explanations make sense? |
|
I am seeing ARM build failure because of commit 2f5f0af, here's the error: |
interfacer linter was introduced in moby#34625. Signed-off-by: David Sheets <dsheets@docker.com>
interfacer linter was introduced in moby#34625. Signed-off-by: David Sheets <dsheets@docker.com>
interfacerchecks function arguments for types that could use a smaller interfaceunconvertfinds unnecessary type convertions