Skip to content

added devfile component changes #147

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

Merged
merged 8 commits into from
Apr 4, 2022

Conversation

jc-berger
Copy link
Contributor

@jc-berger jc-berger commented Mar 7, 2022

Fixes: devfile/api#758

Direct link to preview: https://devfile-docs-147.surge.sh/devfile/2.2.0/user-guide/adding-components-to-a-devfile/

Added the necessary additions to the tables for devfile components. Please let me know what you think!

@jc-berger jc-berger added the documentation Improvements or additions to documentation label Mar 7, 2022
@jc-berger jc-berger self-assigned this Mar 7, 2022
@github-actions
Copy link

github-actions bot commented Mar 7, 2022

🎊 Navigate the preview: https://devfile-docs-147.surge.sh 🎊

@github-actions
Copy link

github-actions bot commented Mar 7, 2022

Click here to review and test in web IDE: Contribute

Copy link
Contributor

@kim-tsao kim-tsao left a comment

Choose a reason for hiding this comment

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

This PR fixes the problems reported in devfile/api#758 (comment) but I think we should consider reorganizing the tables. For example, we have tables for the container and volume components but we're missing a table for the Kubernetes/Openshift component. Rather than have the tables listed in one page, maybe we can move them to their respective topics?

i.e. Container attributes and all its sub-attributes listed under Adding a container component to a devfile, Kubernetes/Openshift attributes listed under Adding a Kubernetes or OpenShift component to a devfile, etc

Keep in mind, some attributes are common across multiple components such as the endpoint object so we can have the table on one page and have a URL link so it can be referred to from other pages.

Let me know if I should open up another issue since this may be a lot of work.

@jc-berger
Copy link
Contributor Author

@kim-tsao good idea. Think we can merge this PR, open a new issue, then do the heavier work for that issue?

Don't want this PR to take too long, especially since we're currently transitioning writers.

Copy link

@max-cx max-cx left a comment

Choose a reason for hiding this comment

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

LGTM

@openshift-ci
Copy link

openshift-ci bot commented Mar 11, 2022

@max-cx: changing LGTM is restricted to collaborators

In response to this:

LGTM

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.

@jc-berger
Copy link
Contributor Author

@rkratky ready for your review, thanks

Copy link
Contributor

@rolfedh rolfedh 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. It looks like you might have missed this "whether or not" issue: #147 (comment)

@openshift-ci
Copy link

openshift-ci bot commented Mar 11, 2022

@rolfedh: changing LGTM is restricted to collaborators

In response to this:

Hi @jc-berger. It looks like you might have missed this "whether or not" issue: #147 (comment)

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.

@jc-berger
Copy link
Contributor Author

jc-berger commented Mar 11, 2022

@rolfedh I think I committed your suggestion, do you see otherwise?

I see in the commit history it's been changed, and I confirmed locally. See the attached screenshots

Screen Shot 2022-03-11 at 1 28 53 PM

Screen Shot 2022-03-11 at 1 33 37 PM

:

@rolfedh
Copy link
Contributor

rolfedh commented Mar 13, 2022

I think I committed your suggestion, do you see otherwise?

Sorry. I must have been looking at outdated information. Approved.

@openshift-ci
Copy link

openshift-ci bot commented Mar 13, 2022

@rolfedh: changing LGTM is restricted to collaborators

In response to this:

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.

@openshift-ci
Copy link

openshift-ci bot commented Apr 4, 2022

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: jc-berger, kim-tsao, max-cx, rkratky, rolfedh

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

@rkratky rkratky merged commit 6d66967 into devfile:master Apr 4, 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 lgtm
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the Adding components to a devfile doc
6 participants