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

Edited and restructured server-api file #4106

Merged
merged 10 commits into from Oct 25, 2022
Merged

Conversation

alwasega
Copy link
Contributor

I have edited and restructured some parts of the page to make it tighter and as per the Diataxis model.

  • I would suggest changing the title of the page to Managing servers using the JupyterHub API and not Starting servers using the JupyterHub API. Starting a server is only one of the steps covered in the article and it does not, therefore, give a comprehensive picture of what the article is about. However, I have not changed the title as doing so might break something else somewhere.
  • Later in the article, I have used Progress API and not progress API, which breaks the sentence case styling of (sub)headings. However, I feel using a proper noun for Progress makes more sense given the context. I am open to corrections on this though.

@minrk minrk added documentation outreachy-dec22 Issues and tasks related to the Dec 22 cohort of Outreachy labels Oct 20, 2022
Copy link
Member

@minrk minrk left a comment

Choose a reason for hiding this comment

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

Thanks for all the great suggestions! I've left some comments inline that I think will improve the final result.

docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
(attempting to access it may not work)
because it isn't finished spawning yet.
We'll get more into that below in [waiting for a server][].
Note that `ready` is assigned `false` and `pending` is assigned `spawn`, meaning that the server is not ready and attempting to access it may not work as it is still in the process of spawning.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure assigned is helpful here. I would still say ready is false. But if you do find we need different language, I would probably pick something like 'has the value'. But in short, I think is is clear and concise and standard language for a field having a value.

Can you speak more to removing the link? It seems useful to keep it here that we're going to get more into that later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have removed assigned as suggested. I have also returned the link as your explanation makes sense. I removed it as I figured the reader will get to the section either way, so it did not add any value having the information.

docs/source/reference/server-api.md Outdated Show resolved Hide resolved

[waiting for a server]: waiting

(starting)=

## Starting servers

To start a server, make the request
To start a server, make the following API request:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To start a server, make the following API request:
To start a server, make the API request:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have used "this" rather than "the"

docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
@alwasega
Copy link
Contributor Author

Thanks for all the great suggestions! I've left some comments inline that I think will improve the final result.

Thanks @minrk . I will update the file accordingly.

@alwasega
Copy link
Contributor Author

Hi @minrk.
Please check commit #9f5f19cb264bcaac334d828367c501abb5e94f8e above.
Thank you.

Copy link
Member

@minrk minrk left a comment

Choose a reason for hiding this comment

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

Great! A few more suggestions for language refinements, then I think it's all set.

docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
docs/source/reference/server-api.md Outdated Show resolved Hide resolved
@alwasega
Copy link
Contributor Author

Hi @minrk. Is commit 3da4dc6 now okay?

@minrk
Copy link
Member

minrk commented Oct 25, 2022

Wonderful, thank you!

@minrk minrk merged commit 297c401 into jupyterhub:main Oct 25, 2022
@alwasega
Copy link
Contributor Author

Thank you too @minrk for your review and guidance. I have truly learnt a lot from this experience.

@alwasega alwasega deleted the server branch October 25, 2022 08:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation outreachy-dec22 Issues and tasks related to the Dec 22 cohort of Outreachy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants