Skip to content

Update CLI docs for release 779/merge #3528

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

Merged
merged 16 commits into from
Apr 29, 2025
Merged

Conversation

github-actions[bot]
Copy link
Contributor

@@ -1,311 +1,961 @@
---
id: activity
title: Temporal CLI activity command reference
sidebar_label: activity
title: temporal activity
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to other reviewers: lower case reflects the CLI name and command

Copy link
Contributor

Choose a reason for hiding this comment

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

I assume we'll want title and sidebar_label to match what we have today?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't want anyone to block you for what might look like a casing mistake

@fairlydurable
Copy link
Contributor

@brianmacdonald-temporal @yuandrew I'm so excited to see this automated PR start to come to life. I haven't checked the preview yet, but we should make sure the front matter YAML is aligned with our current Docs team policies and that the produced output looks like a docs page, not autogen. I'll let @brianmacdonald-temporal make that call.

  • Left a blocking review due to the front matter issue
  • Noted that the non-standard capitalization fits with the CLI invocations

@fairlydurable
Copy link
Contributor

@brianmacdonald-temporal Do you remember for the IA "what if" deployment I rewrote the installing the CLI section? It's here: https://temporal-documentation-git-edu-4213.preview.thundergun.io/build-with-temporal/setup-cli

@yuandrew What do you think about this? Could we use it to replace the install page?

@yuandrew
Copy link
Contributor

@fairlydurable

I haven't checked the preview yet, but we should make sure the front matter YAML is aligned with our current Docs team policies and that the produced output looks like a docs page, not autogen.

What is "front matter YAML" here?

Do you remember for the IA "what if" deployment I rewrote the installing the CLI section? It's here: https://temporal-documentation-git-edu-4213.preview.thundergun.io/build-with-temporal/setup-cli

Sorry, what is the ask here? If the link should replace today's install page? That's probably something better brought up to the CLI group. From a quick look, the new page looks good

@fairlydurable
Copy link
Contributor

@yuandrew Frontmatter is the YAML section of an MDX file that defines the file's metadata. It's at the start of the file and ends with the dashes.

@fairlydurable
Copy link
Contributor

@yuandrew I'll bring up replacing the install with the CLI group. Who would be a good person to approach?

@yuandrew
Copy link
Contributor

@fairlydurable got it. With temporalio/cli#791 in, all front matter should match now!

Copy link
Contributor

@fairlydurable fairlydurable left a comment

Choose a reason for hiding this comment

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

  • Files reviewed
  • Vercel preview inspected
  • Synced via Zoom

@fairlydurable
Copy link
Contributor

Unaligned line continuation; recommended one space at end of line before backslash

image

@fairlydurable
Copy link
Contributor

Seems to have a code fence error

image

@fairlydurable
Copy link
Contributor

Missing /worker link in sidebars

@yuandrew
Copy link
Contributor

@fairlydurable

Unaligned line continuation; recommended one space at end of line before backslash

Where are you seeing this? I'm only seeing code samples with the \ recommended format

@fairlydurable
Copy link
Contributor

@yuandrew I left a screenshot that was rendered from the vercel preview. Let me find the source for you

@fairlydurable
Copy link
Contributor

@yuandrew My apologies. That screenshot is from the live docs site not the preview. Please ignore

Copy link
Contributor

@fairlydurable fairlydurable left a comment

Choose a reason for hiding this comment

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

I am unblocking this, will merge after OK from @yuandrew and a final Vercel inspection

@fairlydurable
Copy link
Contributor

@yuandrew Made sure this was not the live site this time. :)

image

@fairlydurable
Copy link
Contributor

fairlydurable commented Apr 28, 2025

Recommending a global search for +--

image

@fairlydurable
Copy link
Contributor

Consider trimming overlapping content on the landing page wrt the new install / quick start page

@yuandrew
Copy link
Contributor

@fairlydurable we're happy to merge this! We will address the +-- and trimming duplicate setup info between the 2 pages in a follow up PR


- [--workflow-id](/cli/cmd-options#workflow-id)
**--output**, **-o** _string-enum_
Copy link
Contributor

Choose a reason for hiding this comment

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

table with links,

we have inline definitions now,

@fairlydurable fairlydurable merged commit 99a0c0b into main Apr 29, 2025
6 checks passed
@fairlydurable fairlydurable deleted the update-cli-docs-v1.3.0 branch April 29, 2025 17:39
@fairlydurable
Copy link
Contributor

Merged. Looking forward to your follow-ups

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.

2 participants