Update toolkit overview: add components, remove diagram #157
Conversation
jgehrcke
force-pushed
the
jp/toolkit-overview-list-components
branch
from
a61d0d7 to
97693fd
Compare
container-toolkit/index.md
Outdated
| The NVIDIA Container Toolkit enables users to build and run GPU-accelerated containers. | ||
| The toolkit includes a container runtime [library](https://github.com/NVIDIA/libnvidia-container) | ||
| and utilities to automatically configure containers to leverage NVIDIA GPUs. | ||
| The NVIDIA Container Toolkit is a collection of libraries and utilities enabling users to build and run GPU-accelerated containers. |
There was a problem hiding this comment.
- Removed the "includes ..." part of the sentence, because of the new complete enumeration just below.
- I do like the sentence "... to automatically configure containers to leverage NVIDIA GPUs." and I also like "...to build and run GPU-accelerated containers". I think in the overview we do not need both. What we end up with clearly is a matter of taste. I am just proposing some kind of simplification.
|
|
||
| ```{image} https://cloud.githubusercontent.com/assets/3028125/12213714/5b208976-b632-11e5-8406-38d379ec46aa.png |
There was a problem hiding this comment.
I propose removing the diagram. It might be a provocative change. My main argument is that the diagram does not help answering what the toolkit is and where it fits the big picture. In particular, it does not mention: the container toolkit, the runtime library, i.e. it does not really help clarify what the toolkit is and does. It however mentions a different toolkit, and that might be confusing.
That's the diagram:
There was a problem hiding this comment.
I think I'm ok with that. The image looks pretty, but doesn't add real value in this context.
There was a problem hiding this comment.
+1 to removing it if its not right/not adding value.
Is there a diagram that would make sense here? I'm not sure how this image got created, but if there is a diagram we'd like to see here, I can do more investigating into getting them created.
There was a problem hiding this comment.
Is there a diagram that would make sense here?
I think it's ok for today to not investigate creating a new one. We will see. Glad we agree on the removal.
container-toolkit/index.md
Outdated
| ```{image} https://cloud.githubusercontent.com/assets/3028125/12213714/5b208976-b632-11e5-8406-38d379ec46aa.png | ||
| :width: 400 | ||
| ``` | ||
| The Toolkit currently includes: |
There was a problem hiding this comment.
Main proposal: add a concise list of (current) components. One of the best ways to answer "what is this toolkit, really?". Thanks for assembling the list @elezar.
|
|
||
| ## License | ||
|
|
||
| The NVIDIA Container Toolkit (and all included components) is licensed under [Apache 2.0](https://www.apache.org/licenses/LICENSE-2.0) and | ||
| contributions are accepted with a DCO. See the [contributing](https://github.com/NVIDIA/nvidia-container-toolkit/blob/master/CONTRIBUTING.md) document for | ||
| contributions are accepted with a Developer Certificate of Origin (DCO). See the [contributing](https://github.com/NVIDIA/nvidia-container-toolkit/blob/master/CONTRIBUTING.md) document for |
Documentation preview |
| * The NVIDIA CDI Hooks (`nvidia-cdi-hook`) | ||
| * The NVIDIA Container Runtime Hook (`nvidia-container-runtime-hook`) | ||
| * The NVIDIA Container CLI (`nvidia-container-cli`) | ||
| * The NVIDIA Container Library (`libnvidia-container1`) |
There was a problem hiding this comment.
We can think about linking out to the github repos of those components (not sure how we feel about that from within the docs -- are most links in docs expected to be cross-references into other parts in docs?)
There was a problem hiding this comment.
I'm not sure about links to the GitHub repo(s) at this stage as that adds additional questions. For example, the first four components listed here are developed from the nvidia-container-toolkit repo and the last two from libnvidia-container. We are moving towards just needing a single one and maybe we can add the reference then?
There was a problem hiding this comment.
I agree, it might be too much have the links here to each component repo at this point in the docs. If we wanted to include links to the repo here, it might make sense to do that in a table format. So we could include a "Repo link" column in the and visually show that some components are in the same repo.
In general, I'm pro adding links and cross-referencing things. No real restrictions on adding links to github repos or other docs sites (like the k8s.io site) to help explain concepts. As long as its adding value and not chaos. I guess the only note there is if we have docs/explanations in a repo that should live in the docs site, then we should add it to the docs site, instead of linking to the repo from the docs.
There was a problem hiding this comment.
Thanks for the exhaustive feedback to both of you! I will leave this as-is.
No real restrictions on adding links to github repos or other docs sites (like the k8s.io site) to help explain concepts.
Thanks for confirming! :)
container-toolkit/index.md
Outdated
| ```{image} https://cloud.githubusercontent.com/assets/3028125/12213714/5b208976-b632-11e5-8406-38d379ec46aa.png | ||
| :width: 400 | ||
| ``` | ||
| The Toolkit currently includes: |
There was a problem hiding this comment.
nit: We refer to this as its full name when we reference it.
| The Toolkit currently includes: | |
| The Container Toolkit currently includes: |
There was a problem hiding this comment.
Consistency is key, I know and appreciate that.
In that regard, what do we think about "The Container Toolkit" vs "The NVIDIA Container Toolkit"?
The previous sentence starts with "The NVIDIA Container Toolkit" so I thought I don't repeat that.
The table of contents (side bar) shows
so I thought "The Toolkit" might be an allowable term to use.
OK, easy way out: I just use "It" here (using the previous sentence as the reference). Commit upcoming.
- A concise list of components helps new users demystify quickly what the toolkit really is (is comprised of) - The diagram does not mention: the toolkit, the runtime library, i.e. it does not really help clarify what the toolkit is and does. Ot mentions the CUDA Toolkit however and that might be confusing. Signed-off-by: Dr. Jan-Philip Gehrcke <[email protected]> review feedback Signed-off-by: Dr. Jan-Philip Gehrcke <[email protected]>
jgehrcke
force-pushed
the
jp/toolkit-overview-list-components
branch
from
97693fd to
65617f5
Compare
A proposal. I am adding inline comments with reasoning after opening the PR. Feedback appreciated!