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.

Sign up for GitHub

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

Jump to bottom

[docs-beta] migrate - agent-config.md #26480

Merged
merged 9 commits into from
Dec 19, 2024
Merged

[docs-beta] migrate - agent-config.md #26480

merged 9 commits into from
Dec 19, 2024
+262 −8

Conversation

cmpadden
Copy link
Contributor

@cmpadden cmpadden commented Dec 13, 2024 •
edited
Loading

Summary & Motivation

Note that I renamed the file to .mdx since we are using components. I'm not sure what the etiquette is here, so please let me know if that's not correct.

You may find this diff helpful.

$ diff \
    docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx \
    ../content/dagster-plus/managing-deployments/setting-environment-variables-agents.mdx
image

How I Tested These Changes

Changelog

Insert changelog entry or delete this section.

@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 13, 2024 •
edited
Loading

Deploy preview for dagster-docs-beta ready!

Preview available at https://dagster-docs-beta-jhro41cbm-elementl.vercel.app

Direct link to changed pages:

@cmpadden cmpadden marked this pull request as ready for review December 13, 2024 19:28
@cmpadden cmpadden requested a review from neverett as a code owner December 13, 2024 19:28
@cmpadden cmpadden changed the title [docs-beta] migrate agent-config.md [docs-beta] migrate - agent-config.md Dec 15, 2024
@cmpadden cmpadden added the area:docs-revamp Related to the revamp of the docs label Dec 16, 2024
@neverett
Copy link
Contributor

Note that I renamed the file to .mdx since we are using components. I'm not sure what the etiquette is here, so please let me know if that's not correct.

I had assumed this was necessary, but the index pages that use the DocCardList component are just regular .md files, so I don't know what to believe anymore. At any rate, .mdx ensures they will always work, so I'm in favor.

neverett
neverett approved these changes Dec 16, 2024
View reviewed changes
Copy link
Contributor

@neverett neverett 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 formatting fixes needed, otherwise LGTM

docs/docs-beta/docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx Outdated Show resolved Hide resolved
docs/docs-beta/docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx Outdated
```

{/*
<ReferenceTable>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this will just need to be a Markdown table for now.

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 ended up using a vanilla html table since there was nested html in the content. It seems to be pretty decent.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops, just realized that in the legacy docs it's pivoted. Let me update.
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, ended up using markdown tables like you originally suggested. Nice!
image

docs/docs-beta/docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx Outdated Show resolved Hide resolved
docs/docs-beta/docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx Outdated Show resolved Hide resolved
docs/docs-beta/docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx Outdated Show resolved Hide resolved
docs/docs-beta/docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx Outdated Show resolved Hide resolved
docs/docs-beta/docs/dagster-plus/deployment/management/environment-variables/agent-config.mdx Outdated Show resolved Hide resolved
@cmpadden cmpadden merged commit 33ba996 into master Dec 19, 2024
1 of 3 checks passed
@cmpadden cmpadden deleted the colton/doc-605-environment-variables-docs branch December 19, 2024 02:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@graphite-app graphite-app[bot] graphite-app[bot] left review comments

@neverett neverett neverett approved these changes

Assignees
No one assigned
Labels
area:docs-revamp Related to the revamp of the docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cmpadden @neverett