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

Create features.md #17

Merged
merged 9 commits into from
Aug 4, 2022
Merged

Create features.md #17

merged 9 commits into from
Aug 4, 2022

Conversation

bamurtaugh
Copy link
Member

@bamurtaugh bamurtaugh commented Aug 4, 2022

Add the content from https://github.com/devcontainers/spec/blob/main/proposals/devcontainer-features.md.

One to-do is to make the headers into links (i.e. ## <a href="#contribution-approaches" name="contribution-approaches" class="anchor"> Contribution approaches </a>), but that could also be done in a subsequent PR.

@bamurtaugh bamurtaugh requested a review from a team as a code owner August 4, 2022 00:51
@Chuxel
Copy link
Member

Chuxel commented Aug 4, 2022

Yeah I think this would be reasonable to add at this point along with a nav! We can then link to it from the devcontainer.json reference and the main spec. A few thoughts:

  1. Maybe we should add [Proposal] to the heading with a brief statement saying something like "This section provides information on a currently active proposal. See Dev Container Features spec#61 for input and links to other proposed improvements. "?
  2. It looks like this references the distribution proposal directly. We may want to pull that file in at the same time or adjust the link to point to the markdown.
  3. We could reference the features section from the main spec I think just by adding a heading under "Other options". E.g., "Dev container 'features' are self-contained, shareable units of installation code and development container configuration. They are applied to container images as a secondary build step and can affect a number of dev container configuration settings. See for more details."
  4. In the "features" property in the devcontainer.json reference, we can reference the "devcontainer.json properties" section of this doc for more info at this point.

@Chuxel Chuxel self-requested a review August 4, 2022 01:21
@bamurtaugh
Copy link
Member Author

Thanks for the great feedback, @Chuxel!

I believe I addressed the 4 points above:

  1. Added the word 'Proposal' and a sentence of context to the features doc
  2. Pulled in the features distribution doc directly, also using 'Proposal' and the context sentence above
  3. Added a features section under "Other Options" in the main spec, linking to the new features.md
  4. Added a link to features.md to the features section of the devcontainer.json reference

I also made all the headers in the features and features distribution docs into links for easier sharing and navigation, consistent with the other pages of the site.

Please let me know if the changes are in-line with what you were thinking.

Copy link
Member

@Chuxel Chuxel left a comment

Choose a reason for hiding this comment

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

A few suggested tweaks.

_implementors/features.md Outdated Show resolved Hide resolved
_implementors/json_reference.md Outdated Show resolved Hide resolved
_implementors/spec.md Outdated Show resolved Hide resolved
bamurtaugh and others added 3 commits August 4, 2022 07:49
Co-authored-by: Chuck Lantz <clantz@microsoft.com>
Co-authored-by: Chuck Lantz <clantz@microsoft.com>
Co-authored-by: Chuck Lantz <clantz@microsoft.com>
@bamurtaugh
Copy link
Member Author

Thanks for the suggestions! Opened devcontainers/spec#69 with the Features wording suggestion you mentioned.

I'll merge these changes in as it seems this is a good baseline, and we can continue with other changes folks might have in mind in follow-up PRs.

@bamurtaugh bamurtaugh merged commit 6038213 into gh-pages Aug 4, 2022
@bamurtaugh bamurtaugh deleted the bamurtaugh-features branch August 5, 2022 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants