Skip to content
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

WIP: rewrite runAttach to be exported #4643

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

thaJeztah
Copy link
Member

- Description for the changelog

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

Stderr: true,
DetachKeys: detachKeys,
if opts.DetachKeys == "" {
opts.DetachKeys = dockerCli.ConfigFile().DetachKeys
Copy link
Member Author

Choose a reason for hiding this comment

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

Ideally, we'd move this elsewhere so that this function can potentially only depend on the API-client, not the CLI 🤔

@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2023

Codecov Report

Merging #4643 (d92821d) into master (a1bd689) will increase coverage by 0.00%.
The diff coverage is 46.51%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4643   +/-   ##
=======================================
  Coverage   59.58%   59.59%           
=======================================
  Files         287      285    -2     
  Lines       24722    24730    +8     
=======================================
+ Hits        14731    14737    +6     
  Misses       9106     9106           
- Partials      885      887    +2     

@thaJeztah thaJeztah force-pushed the attach_pass_container_arg branch 2 times, most recently from 0000946 to d92821d Compare January 26, 2024 12:46
Trying to make "runAttach" not depend on attachOptions, and to see if we
can make it accept container.AttachOptions instead

relates to docker#4637

Signed-off-by: Sebastiaan van Stijn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants