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

Modified language to adhere better to IBM style guide #67

Merged
merged 10 commits into from
Jun 17, 2021
Merged

Modified language to adhere better to IBM style guide #67

merged 10 commits into from
Jun 17, 2021

Conversation

jc-berger
Copy link
Contributor


Fixes #254 Review Language in Authoring Stacks.Since I am doing a review of all the authoring stacks docs, I'll be making several PRs. I don't want any single PR to be too large, so I'll make a new PR for every several docs that I modify to meet the IBM Style Guide's requirements.

@jc-berger jc-berger added the documentation Improvements or additions to documentation label May 18, 2021
@jc-berger jc-berger self-assigned this May 18, 2021
Copy link
Contributor

@elsony elsony left a comment

Choose a reason for hiding this comment

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

/lgtm

@jc-berger
Copy link
Contributor Author

@rkratky ready to merge if you think it looks good, thanks!

Copy link

@rkratky rkratky left a comment

Choose a reason for hiding this comment

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

@jc-berger, comments submitted.

@@ -78,7 +78,7 @@ The sources is mounted on a location stored in the `PROJECTS_ROOT` environment v

. Container Entrypoint
+
The `command` attribute of the `container` along with other arguments, is used to modify the `entrypoint` command of the container created from the image. In {prod} the container is needed to run indefinitely so that you can connect to it and execute arbitrary commands in it at any time. Because the availability of the `sleep` command and the support for the `infinity` argument for it is different and depends on the base image used in the particular images, {prod-short} cannot insert this behavior automatically on its own. However, you can take advantage of this feature to, for example, start up necessary servers with modified configurations, etc.
Use the `command` attribute of the `container` to modify the `entrypoint` command of the container created from the image. In devfiles, the container runs indefinitely, so you can consistently connect to it and execute commands in it. The availability of the `sleep` command and the support for the `infinity` argument depend on the base image used in the particular images. Therefore, devfiles cannot automatically include these commands.
Copy link

Choose a reason for hiding this comment

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

  • Meaning changed. The orig. use of the {prod} attribute was incorrect, but it wasn't right to substitute it for devfiles (containers don't "run in devfiles").
  • [IMPORTANT] Given the orig. content's orientation on Eclipse Che, the validity/suitability of this example needs to be checked with SMEs (esp. the inclusion and explanation of the sleep and infinity commands).
Suggested change
Use the `command` attribute of the `container` to modify the `entrypoint` command of the container created from the image. In devfiles, the container runs indefinitely, so you can consistently connect to it and execute commands in it. The availability of the `sleep` command and the support for the `infinity` argument depend on the base image used in the particular images. Therefore, devfiles cannot automatically include these commands.
Use the `command` attribute of the `container` type to modify the `entrypoint` command of the container created from the image. When a devfile is used in {che-prod}, the container runs indefinitely, so the user can consistently connect to it and execute commands in it. The availability of the `sleep` command and the support for the `infinity` argument depend on the base image used in the particular images.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@maysunfaisal , apologies for not directing your attention to this specific comment earlier. Given Robert's question, does the above example still work, specifically the explanation of the sleep and infinity command? Thanks!

@@ -36,7 +36,7 @@ parent:
+
.Parent referred by uri
Copy link

Choose a reason for hiding this comment

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

Suggested change
.Parent referred by uri
.Parent referred by a URI

Copy link

Choose a reason for hiding this comment

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

Using "URI" (and the associated uri attribute) seems strange here. Shouldn't it be "URL"? If yes, then the spec is wrong :( Confirm with SMEs, please.

Copy link
Member

Choose a reason for hiding this comment

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

it is URI. URI vs URL FYI :)

Copy link

Choose a reason for hiding this comment

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

@maysunfaisal, that's exactly why I was asking. The example uses a URL, not a URI. Would it really be possible to use URIs, or are URLs required?

jc-berger and others added 9 commits June 1, 2021 11:30
…nt-to-a-devfile.adoc

Co-authored-by: Robert Krátký <rkratky@redhat.com>
…nt-to-a-devfile.adoc

Co-authored-by: Robert Krátký <rkratky@redhat.com>
…nt-to-a-devfile.adoc

Co-authored-by: Robert Krátký <rkratky@redhat.com>
…nt-to-a-devfile.adoc

Co-authored-by: Robert Krátký <rkratky@redhat.com>
….adoc

Co-authored-by: Robert Krátký <rkratky@redhat.com>
….adoc

Co-authored-by: Robert Krátký <rkratky@redhat.com>
….adoc

Co-authored-by: Robert Krátký <rkratky@redhat.com>
@jc-berger
Copy link
Contributor Author

Thanks @maysunfaisal for the insight :)

@rkratky please review and see if you think it can be merged now. Thank you both!

@jc-berger
Copy link
Contributor Author

For more work on these Eclipse Che related content concerns, please see issue #491: devfile/api#491

@openshift-ci openshift-ci bot added the lgtm label Jun 17, 2021
@openshift-ci
Copy link

openshift-ci bot commented Jun 17, 2021

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: elsony, jc-berger, rkratky

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 15fb039 into devfile:master Jun 17, 2021
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.

4 participants