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

Add documentation for exec websocket #9679

Merged
merged 18 commits into from
Jan 8, 2021

Conversation

backspace
Copy link
Contributor

@backspace backspace commented Dec 18, 2020

This is a continuation of #6604. You can preview it in situ here.

@vercel vercel bot temporarily deployed to Preview – nomad-storybook-and-ui December 18, 2020 18:00 Inactive
@backspace backspace marked this pull request as ready for review December 18, 2020 19:23
@backspace backspace requested a review from notnoop December 18, 2020 19:24
@backspace backspace added the theme/docs Documentation issues and enhancements label Dec 18, 2020
Copy link
Contributor

@DingoEatingFuzz DingoEatingFuzz left a comment

Choose a reason for hiding this comment

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

I read through it and it makes sense to me!

The thing that wasn't clear to me was if the response frames should be appended to output or if they represent the full buffer.

I know things like top work but it wasn't clear what the mechanics are for line clearing. Maybe those are just xterm instructions that come through the stdout messages?

Comment on lines 791 to 793
# basic application-level heartbeat
{}

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra newline (I tried to get rid of it with a suggestion but it didn't seem to work).

@backspace
Copy link
Contributor Author

Maybe those are just xterm instructions that come through the stdout messages?

This is correct! Do you think this is worth mentioning?

@DingoEatingFuzz
Copy link
Contributor

I don't think it would hurt to add it.

This prevents having to scroll horizontally.
@backspace
Copy link
Contributor Author

I don't think it would hurt to add it.

👍 I added an example request and response to provide some explanation of the content of frames, thoughts?

Copy link
Contributor

@DingoEatingFuzz DingoEatingFuzz left a comment

Choose a reason for hiding this comment

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

I like it! That sample gives just enough guidance on what to expect while also introducing some vocab someone can do further research on.

:shipit:

@backspace backspace merged commit 017b47d into master Jan 8, 2021
@backspace backspace deleted the d-doc-alloc-exec-websocket-redux branch January 8, 2021 20:01
backspace added a commit that referenced this pull request Jan 22, 2021
@github-actions
Copy link

github-actions bot commented Dec 4, 2022

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 4, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
theme/docs Documentation issues and enhancements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants