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.

Sign up for GitHub

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

Doc update for life cycle hook features #348

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Doc update for life cycle hook features #348

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions docs/specs/devcontainer-features.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,13 @@ All properties are optional **except for `id`, `version`, and `name`**.

The following lifecycle hooks may be declared as properties of `devcontainer-feature.json`.

| Property | Type|
| :--- | :--- |
| `onCreateCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties)|
| `updateContentCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties)|
| `postCreateCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties)|
| `postStartCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties) |
| `postAttachCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties) |
| Property | Type|Description |
| :--- | :--- | :-- |
| `onCreateCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties)|First time inside the container immediately after it has started. |
| `updateContentCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties)| First time inside the container after `onCreateCommand` whenever new content is available in the source tree during the creation process.|
Copy link
Member

Choose a reason for hiding this comment

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

@joshspicer do you think we should use the wordings consistent to the documentation here

Copy link
Member

Choose a reason for hiding this comment

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

Would a link back to https://containers.dev/implementors/json_reference/#lifecycle-scripts on this page be sufficient clarity, @dfberry ?

I think reducing the amount of duplicated descriptions would be helpful to prevent information getting stale.

Regardless I agree on the consistency

Copy link
Author

Choose a reason for hiding this comment

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

There is a link back, it below but not super obvious. I tried to be very respectful of the text in the other doc. I didn't know these other life cycle hooks where available because dev containers in vscode really only shows 1 by default. I was trying to make it more obvious but perhaps this is the wrong platform for that? Not sure. @seesharprun @bamurtaugh Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for opening this @dfberry. I love the goal @joshspicer mentions to avoid duplicated definitions and include links as possible, especially as we have content here and in containers.dev.

@dfberry maybe if the link wasn't apparent, another link could be added on top of the table, and/or links could be added to each entry in the table?

And speaking of containers.dev, for any change we pursue in this repo, could you also open a PR in the containers.dev repo when you get a chance: https://github.com/devcontainers/devcontainers.github.io/blob/gh-pages/_implementors/features.md? Thank you!

Copy link
Author

@dfberry dfberry Dec 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@bamurtaugh Let me step back and walk through how I got here because I definitely think a link is missing from the devcontainer created by VSCode to this understanding of the different lifecycle events which @seesharprun alerted me to. I only knew about the 1 hook in the typical TS/JS/Node.js container. Once I got to this documentation for lifecycle properties, I had to figure out which lifecycle event was best suited to the typical usecases for scripts in the dev container.

Issues are:

  1. from devcontainers, how do I know and understand their are more lifecycle hooks than the 1 presented in the devcontainer
  2. once I get to the documentation, how do I understand use case for each event

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for clarifying. It sounds like an alternative option might be adding a link to these docs via a comment in the dev container Template or image, i.e. above this line https://github.com/devcontainers/templates/blob/main/src/cpp/.devcontainer/devcontainer.json#L16. Does that sound like it would've helped @dfberry?

| `postCreateCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties)| First time after `updateContentCommand` and once the dev container has been assigned to a user.|
| `postStartCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties) |Every time the container is successfully started.|
| `postAttachCommand` | [string, array, object](/docs/specs/devcontainerjson-reference.md#formatting-string-vs-array-properties) |Every time a tool has successfully attached to the container.|

#### Behavior

Expand Down