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

Initial devcontainer CLI docs #4698

Merged
merged 4 commits into from
Jul 28, 2021

Conversation

stuartleeks
Copy link
Contributor

Adds initial docs for the devcontainer CLI:

I made an initial estimate of placement in the table of contents, but let me know if the order should be tweaked

Copy link
Member

@bamurtaugh bamurtaugh left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for writing this up! I definitely learned by reading this 😄. Left a few pieces of feedback, please let me know if they do or don't make sense.

docs/remote/devcontainer-cli.md Outdated Show resolved Hide resolved
* **Windows**: You will be prompted to automatically add the `devcontainer` CLI to your `PATH` or to copy the `devcontainer` CLI path to your clipboard for you to add to your `PATH`.
* **macOS/Linux**: If the extension detects a `bin` folder ( or `.local/bin` folder) in your user home folder and in your `PATH` then you will have the option of adding a symlink to the `devcontainer` CLI to this location. You will also have the option to copy the `devcontainer` CLI path to your clipboard for you to add to your `PATH`.

3. From a system shell (one not inside Visual Studio Code), run `devcontainer --help` to test the installation and see the CLI's built-in help. Note that you may need to restart your shell for `PATH` changes to take effect.
Copy link
Member

Choose a reason for hiding this comment

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

What do we mean by system shell here? An external terminal?

Copy link
Member

Choose a reason for hiding this comment

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

It could be a nice touch to include a screenshot of the devcontainer ---help commands. Or if we think they may be changing frequently, we could leave it off if it feels too difficult to maintain/it would become more distracting than helpful :)

Copy link
Member

Choose a reason for hiding this comment

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

As I continued reading through the doc, I realized a list of commands (perhaps even a text box / markdown table rather than screenshot) could be helpful so that I know what I can do with the CLI going into this.

Copy link
Contributor Author

@stuartleeks stuartleeks Jul 27, 2021

Choose a reason for hiding this comment

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

Changed the wording for system shell
Added a code block with the devcontainer --help output - that feels like it would be simpler to maintain but hopefully still fulfill the intended purpose. What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I really like it! I think text is easier to maintain too, and showing users upfront what the commands should be is nice in case they don't get the expected output or they are just reading for now and not following along live.

docs/remote/devcontainer-cli.md Outdated Show resolved Hide resolved
docs/remote/devcontainer-cli.md Outdated Show resolved Hide resolved
docs/remote/devcontainer-cli.md Outdated Show resolved Hide resolved
docs/remote/devcontainer-cli.md Show resolved Hide resolved
@stuartleeks
Copy link
Contributor Author

Thanks @bamurtaugh - great feedback. I've resolved most of the issues, but left one open to see what you think of the change.

@gregvanl gregvanl self-assigned this Jul 27, 2021
Copy link
Member

@bamurtaugh bamurtaugh left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for incorporating the feedback so quickly @stuartleeks!

Not sure if you may want a glance over from @Chuxel too, but LGTM.

@gregvanl gregvanl merged commit 71697a5 into microsoft:vnext Jul 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants