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

info: add CDI spec directories to output #4510

Merged
merged 1 commit into from
Aug 28, 2023

Conversation

thaJeztah
Copy link
Member

- Description for the changelog

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

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@codecov-commenter
Copy link

Codecov Report

Merging #4510 (1c2cc4b) into master (cdabfa2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4510   +/-   ##
=======================================
  Coverage   59.38%   59.39%           
=======================================
  Files         288      288           
  Lines       24783    24788    +5     
=======================================
+ Hits        14717    14722    +5     
  Misses       9179     9179           
  Partials      887      887           

Copy link
Contributor

@elezar elezar left a comment

Choose a reason for hiding this comment

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

Thanks @thaJeztah

I completely forgot to pick this up once the moby changes were merged.

One thing that I was just thinking, where's the best place to document this setting? I should create a PR for that too.

@thaJeztah
Copy link
Member Author

I completely forgot to pick this up once the moby changes were merged.

Ha! I found a tab open in one of my browser-windows, and thought "let me do this, before I forget"

One thing that I was just thinking, where's the best place to document this setting? I should create a PR for that too.

Yes, we probably should have a page somewhere with a more extensive example showing how to use CDI in general (also outlining that it's still experimental, and things may change; perhaps a feedback link to a discussion in moby/moby where people can discuss the feature).

Perhaps @dvdksn has ideas what the best location would be for that

@dvdksn
Copy link
Contributor

dvdksn commented Aug 22, 2023

happy to help figure out how to document this. But prepare yourself @thaJeztah because I have questions :)

@thaJeztah
Copy link
Member Author

Ha! I'm prepared, but I think @elezar may be able to assist as well. I guess for the docs we'd want to have some very-basic example how to set up a CDI spec (something just to illustrate the concept).

For Docker Desktop, we'll probably run into some "fun", but we can keep that for a later exercise (we need to think if (and how) we could support it there, because it requires things to be available inside the VM, but perhaps we can come up with a solution for that.

@elezar
Copy link
Contributor

elezar commented Aug 23, 2023

Yes, I can start the PR if you could point me to the best location for the additional information?

@dvdksn
Copy link
Contributor

dvdksn commented Aug 23, 2023

For now, I'd consider this page the "most correct" location for this information. The source for this page is in the CLI repository, here.

WDYT @thaJeztah ?

I started creating a section dedicated to "using the CLI", covering various flags and how to use them. I think that could've been a good place for this but that section is on hold for the moment.

@elezar
Copy link
Contributor

elezar commented Aug 25, 2023

@dvdksn @thaJeztah I have created #4525 as a placeholder for the documentation changes. This currently only references the run command, but we should also add context to the info command.

@elezar elezar mentioned this pull request Aug 25, 2023
5 tasks
@thaJeztah
Copy link
Member Author

Thanks! Let me bring this one in; after that we can work on #4525

@thaJeztah thaJeztah merged commit dfca19a into docker:master Aug 28, 2023
74 checks passed
@thaJeztah thaJeztah deleted the info_cdi_dirs branch August 28, 2023 10:19
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.

None yet

5 participants