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

[Docs] Many docs improvements #2171

Merged
merged 14 commits into from
Jan 8, 2025
Merged

Conversation

peterschmidt85
Copy link
Contributor

Fixes #2170

@peterschmidt85 peterschmidt85 linked an issue Dec 31, 2024 that may be closed by this pull request
7 tasks
docs/docs/guides/server-deployment.md Outdated Show resolved Hide resolved
docs/docs/reference/dstack.yml/task.md Outdated Show resolved Hide resolved
docs/docs/reference/dstack.yml/dev-environment.md Outdated Show resolved Hide resolved
eos_token: "</s>"
```

##### Limitations
Copy link
Collaborator

Choose a reason for hiding this comment

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

Limitations is now part of the ToC because of toc_depth: 5.

Screenshot From 2025-01-03 13-41-26

I don't think it's supposed to be there, especially considering that clicking it doesn't work unless the TGI tab is open.

@@ -1187,36 +276,127 @@ See the [reference table](#default-permissions) for all configurable permissions
cat my-service-account-file.json | jq -c | jq -R
```

## `projects[n].backends[type=kubernetes].networking` { #_networking data-toc-label="networking" }
###### `projects[n].backends[type=kubernetes].networking` { #kuberentes-networking data-toc-label="networking" }
Copy link
Collaborator

Choose a reason for hiding this comment

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

This typo leads to an invalid anchor link.

Suggested change
###### `projects[n].backends[type=kubernetes].networking` { #kuberentes-networking data-toc-label="networking" }
###### `projects[n].backends[type=kubernetes].networking` { #kubernetes-networking data-toc-label="networking" }

Comment on lines +317 to +318
> The `dstack` server allows you to configure backends for multiple projects.
> If you don't need multiple projects, use only the `main` project.
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) This note looks out of place here. Consider moving to the top of the reference or removing.

`dstack` can provision and manage compute across a variety of providers.

To use `dstack` with specific providers, configure backends in the
`~/.dstack/server/config.yml` file before starting the server.
Copy link
Collaborator

Choose a reason for hiding this comment

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

(nit) Consider linking this page to the reference, e.g. here.

Suggested change
`~/.dstack/server/config.yml` file before starting the server.
[`~/.dstack/server/config.yml`](../reference/server/config.yml.md) file before starting the server.

@peterschmidt85 peterschmidt85 requested a review from jvstme January 4, 2025 20:54
@peterschmidt85
Copy link
Contributor Author

@jvstme Pushed more changes to ensure concepts pages include all important examples

# Conflicts:
#	docs/docs/concepts/fleets.md
#	docs/docs/dev-environments.md
#	docs/docs/services.md
#	docs/docs/tasks.md
Copy link
Collaborator

@jvstme jvstme left a comment

Choose a reason for hiding this comment

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

@peterschmidt85, everything looks good overall, but there are quite a few broken links, typos, and a couple of incorrect statements. Hope you can go through my comments before merging. Note that some comments from my previous review are still unresolved as well.

Feel free to ignore the nitpicking comments and let me know if you need any help fixing the links, etc.

I'd also greatly appreciate smaller PRs.

docs/docs/concepts/gateways.md Outdated Show resolved Hide resolved
docs/docs/concepts/gateways.md Outdated Show resolved Hide resolved
docs/docs/concepts/backends.md Outdated Show resolved Hide resolved
docs/docs/concepts/volumes.md Outdated Show resolved Hide resolved
@@ -0,0 +1,30 @@
### Creation policy
Copy link
Collaborator

Choose a reason for hiding this comment

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

The ## header and the fleets definition are missing. They were present previously and seem informative.

Suggested change
### Creation policy
## Manage fleets
Fleets are groups of cloud instances or SSH machines that you use to run dev environments, tasks, and services.
You can let `dstack apply` provision fleets or [create and manage them directly](fleets.md).
### Creation policy

Copy link
Contributor Author

Choose a reason for hiding this comment

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

manage-fleets.ext is included in dev environments, tasks, and services. That's why the header and fleet definitions are intentionally skipped.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why not include it with the ## Manage fleets header? The way you do in manage-runs.ext, which does have the ## Manage runs header.

The ## Manage fleets header was present in the dev-envs, tasks, and services pages previously but was lost in this PR.

For example, this is what the Tasks page used to look like:
Screenshot From 2025-01-08 17-52-28

And this is what it looks like now:
Screenshot From 2025-01-08 17-53-18

Are you sure this is intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At least from my side, this was intentional - to remove "Manage fleets" from these pages. The idea is not to have a dedicated fleet section. We don't have for volumes right? We only mention briefly fleet related options because they relate to "Run a configuration".

docs/docs/concepts/services.md Outdated Show resolved Hide resolved
docs/docs/concepts/services.md Outdated Show resolved Hide resolved
docs/docs/concepts/services.md Outdated Show resolved Hide resolved
docs/docs/concepts/tasks.md Outdated Show resolved Hide resolved
docs/docs/concepts/services.md Outdated Show resolved Hide resolved
@peterschmidt85
Copy link
Contributor Author

@jvstme Fixed all except one (see above). If all is OK, I'm happy to merge. OK?

@peterschmidt85 peterschmidt85 merged commit 2135175 into master Jan 8, 2025
13 of 24 checks passed
@peterschmidt85 peterschmidt85 deleted the 2170-docs-many-docs-improvements branch January 8, 2025 16:34
@jvstme
Copy link
Collaborator

jvstme commented Jan 8, 2025

Fixed all except one

@peterschmidt85, not quite, there are 21 more unresolved comments.

@peterschmidt85
Copy link
Contributor Author

peterschmidt85 commented Jan 8, 2025

Fixed all except one

@peterschmidt85, not quite, there are 21 more unresolved comments.

Okay, seems like the PR is too messy. I will fix them in a separate commit

@peterschmidt85
Copy link
Contributor Author

@jvstme
Pushed via a9356ba

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.

[Docs] Many docs improvements
2 participants