Skip to content

Conversation

@robmry
Copy link
Contributor

@robmry robmry commented Oct 26, 2025

- What I did

- How I did it

- How to verify it

- Human readable description for the release notes

- A picture of a cute animal (not mandatory but encouraged)

@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2025

Codecov Report

❌ Patch coverage is 31.11111% with 31 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
cli/command/container/exec.go 26.66% 10 Missing and 1 partial ⚠️
cli/command/container/start.go 0.00% 11 Missing ⚠️
cli/command/container/inspect.go 0.00% 4 Missing ⚠️
cli/command/system/inspect.go 0.00% 4 Missing ⚠️
cli/command/container/attach.go 80.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@robmry robmry force-pushed the bump_modules_again branch 3 times, most recently from 7361ea6 to de926ca Compare October 26, 2025 17:34
if err != nil {
return nil, nil, err
}
return &res.Container, nil, nil
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
return &res.Container, nil, nil
return &res.Container, res.Raw, nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed, thank you!

@robmry robmry force-pushed the bump_modules_again branch from de926ca to 8a67307 Compare October 26, 2025 18:27
Comment on lines +169 to +175
var cs client.ConsoleSize
if execOptions.ConsoleSize != nil {
cs = client.ConsoleSize{
Height: execOptions.ConsoleSize[0],
Width: execOptions.ConsoleSize[1],
}
}
Copy link
Member

Choose a reason for hiding this comment

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

I'll have a look in a follow-up to see if we actually still need the locally defined execOptions struct / type, or if it has all the same fields as client.ExecAttachOptions (so possibly we could use that one direct).

Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Sebastiaan van Stijn <[email protected]>
@thaJeztah thaJeztah marked this pull request as ready for review October 27, 2025 19:34
@thaJeztah thaJeztah added this to the 29.0.0 milestone Oct 27, 2025
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@thaJeztah thaJeztah merged commit dafbfc2 into docker:master Oct 27, 2025
93 checks passed
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.

3 participants