Skip to content

Conversation

jc-berger
Copy link
Contributor

@jc-berger jc-berger added the documentation Improvements or additions to documentation label Jan 26, 2022
@jc-berger jc-berger self-assigned this Jan 26, 2022
@openshift-ci
Copy link

openshift-ci bot commented Jan 26, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jc-berger

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@github-actions
Copy link

github-actions bot commented Jan 26, 2022

🎊 PR Preview has been successfully built and deployed to https://devfile-docs-preview-pr-132.surge.sh 🎊

= Adding `volume` components to a devfile

[role="_abstract"]
Use volume components in order to share files among container components, making it easier to collaborate with other teams during the development process.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to give a why they'd care about sharing files across container components. What do y'all think of this why I care statement at the end of this abstract?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is very good information to provide. It speaks directly to the user story.

+
.A minimal `volume` component
[source,yaml]
----
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifically for devs, please verify the accuracy and correctness of this source yaml :)

Copy link
Contributor

Choose a reason for hiding this comment

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

This is correct


* Define a component using the type `volume`.
+
.A minimal `volume` component
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this example the user's primary source of information about volume components? If so, you might want to cover any volume-related attributes in this example or topic, for example, by using callouts. Otherwise, consider providing a cross-reference to the reference topic where this information is available.

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay to use monospaced fonts in headings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dev review: do you have suggestion to add to to this yaml sample to address Rolfe's points above?

= Adding a volume component to a devfile

[role="_abstract"]
You can use a volume component in order to share files among container components, making it easier to collaborate with other teams during the development process.
Copy link
Contributor

@rolfedh rolfedh Jan 28, 2022

Choose a reason for hiding this comment

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

IBM SG: For In order to, "Use "to" in most cases to avoid wordiness."

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi, @jake Berger. I was revisiting this PR and realized I missed something important during my peer review. The content talks about adding a component to a devfile, but omits the effect of this action in OpenShift, and then describes the benefit. I'd recommend something more like:

Suggested change
You can use a volume component in order to share files among container components, making it easier to collaborate with other teams during the development process.
To create a <name the type of volume> in your shared <location of volume>, you can add a volume component to your devfile. This <type of volume> makes it easier for teams to share files and collaborate with each other during the development process.

WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

A similar approach might also apply to docs/modules/user-guide/partials/proc_adding-a-kubernetes-or-openshift-component-to-a-devfile.adoc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Devs, I think Rolfe's suggestion is good, but I honestly wouldn't be able to say what type of component it is. Could any of you give an idea to expand our abstract more?

Copy link

@Srivaralakshmi Srivaralakshmi left a comment

Choose a reason for hiding this comment

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

Hi @jc-berger: Nice work! I have left some suggestions. PTAL. Thanks!

@openshift-ci
Copy link

openshift-ci bot commented Jan 31, 2022

@Srivaralakshmi: changing LGTM is restricted to collaborators

In response to this:

Hi @jc-berger: Nice work! I have left some suggestions. PTAL. Thanks!

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.


[role="_abstract"]
This section describes how to add either a `kubernetes` or `openshfit` component to a devfile. You can apply configurations to your devfile with `kubernetes` or `openshift` components.
You can add either a `kubernetes` or `openshfit` component to a devfile. Doing so enables you to configure your devfile to your unique development needs.
Copy link
Contributor

Choose a reason for hiding this comment

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

openshfit is misspelled

. Provide the content through the `uri` or `inlined` property.
+
.Adding `openshift` component using the `uri` property
.Adding a `openshift` component using the `uri` property
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be an ?

. For each component, define an unique value for the mandatory `name` attribute.

. For each component, define a mandatory type of one of the following types: `kubernetes`, `container` or `volume`.
. For each component, define a mandatory type of one of the following types: `kubernetes`, `container`, or `volume`.
Copy link
Contributor

Choose a reason for hiding this comment

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

we should include openshift as one of the components in this list

@jc-berger
Copy link
Contributor Author

closing this PR and continuing the work and conversation in this similar, but updated, PR: #135

@jc-berger jc-berger closed this Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Complete list of components

4 participants