Migrate tutorials #1870
Closed
Migrate tutorials #1870
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…610b29613f251543ab8a37d11c7d68020f41f9
- Create deploy-docs.yml workflow to build and deploy MyST docs - Trigger on main branch changes to tutorials/ directory - Configure MyST for docs.commontools.dev custom domain - Add CNAME file for GitHub Pages custom domain - Document build process in README_BUILD.md - Update MyST config with project metadata and base URL The workflow uses Node.js for MyST builds (separate from Deno codebase) and deploys static documentation to GitHub Pages automatically. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add migrate-tutorials branch to workflow triggers - Enable workflow for pull requests (build-only) - Restrict deployment to push events on main and migrate-tutorials - PRs will build but not deploy (validation only) This allows testing the deployment from the migration branch before merging. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
10 issues found across 35 files
Prompt for AI agents (all 10 issues)
Understand the root cause of the following 10 issues and fix them.
<file name="tutorials/cts.md">
<violation number="1" location="tutorials/cts.md:9">
Fix the typo in the abstract: "succint" should be "succinct".</violation>
</file>
<file name="tutorials/common_ui.md">
<violation number="1" location="tutorials/common_ui.md:266">
`ct-screen` only exposes `header` and `footer` named slots, so using `slot="top"` here prevents the toolbar from rendering. Please use the `header` slot instead.</violation>
<violation number="2" location="tutorials/common_ui.md:274">
`ct-spacer` hasn’t been implemented yet, so this example should use an available spacer alternative to avoid a broken sample.</violation>
<violation number="3" location="tutorials/common_ui.md:310">
The keyboard component is named `ct-kbd`, not `ct-kdb`; please fix the typo so the docs reference the real element.</violation>
</file>
<file name="tutorials/state.md">
<violation number="1" location="tutorials/state.md:27">
Add the missing `);` so the second code sample remains valid TypeScript; otherwise readers copying it will hit a syntax error.</violation>
</file>
<file name="tutorials/myst.yml">
<violation number="1" location="tutorials/myst.yml:8">
The project.github link targets jakedahn/common, so the “Edit on GitHub” links generated by MyST will go to the wrong repository. Update this to the current commontoolsinc/labs/tutorials path so docs readers aren’t sent to the wrong repo.</violation>
</file>
<file name="tutorials/README.md">
<violation number="1" location="tutorials/README.md:3">
Running `myst` without a subcommand just prints the CLI help, so this quickstart won’t launch the local docs server. Replace it with `myst start` to match the working command used in the project scripts.</violation>
</file>
<file name="tutorials/package.json">
<violation number="1" location="tutorials/package.json:7">
The `clean` script uses `|| true` to swallow errors, but `true` is a POSIX utility and is missing on Windows, causing `npm run clean` to fail there. Replace the fallback with a cross-platform no-op such as `node -e "process.exit(0)"`.</violation>
</file>
<file name="tutorials/README_BUILD.md">
<violation number="1" location="tutorials/README_BUILD.md:12">
This bullet implies that only pushes to main deploy the docs, but the workflow also deploys when migrate-tutorials is pushed. Please update the documentation to mention both branches.</violation>
</file>
<file name=".github/workflows/deploy-docs.yml">
<violation number="1" location=".github/workflows/deploy-docs.yml:54">
Insert an actions/configure-pages step before uploading the artifact; otherwise actions/upload-pages-artifact will fail because Pages was never configured.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| keywords: commontools, recipes, cts, types | ||
| abstract: | | ||
| The CTS (Common Type System) helps automatically convert types and common | ||
| coding patterns for you. This leads to more readable and succint code. |
Contributor
There was a problem hiding this comment.
Fix the typo in the abstract: "succint" should be "succinct".
Prompt for AI agents
Address the following comment on tutorials/cts.md at line 9:
<comment>Fix the typo in the abstract: "succint" should be "succinct".</comment>
<file context>
@@ -0,0 +1,15 @@
+keywords: commontools, recipes, cts, types
+abstract: |
+ The CTS (Common Type System) helps automatically convert types and common
+ coding patterns for you. This leads to more readable and succint code.
+ In this section, we will go over the basic features offered by CTS.
+---
</file context>
Suggested change
| coding patterns for you. This leads to more readable and succint code. | |
| coding patterns for you. This leads to more readable and succinct code. |
| <ct-hstack gap="sm"> | ||
| <icon>question</icon> | ||
| <button>hello</button> | ||
| <ct-spacer /> |
Contributor
There was a problem hiding this comment.
ct-spacer hasn’t been implemented yet, so this example should use an available spacer alternative to avoid a broken sample.
Prompt for AI agents
Address the following comment on tutorials/common_ui.md at line 274:
<comment>`ct-spacer` hasn’t been implemented yet, so this example should use an available spacer alternative to avoid a broken sample.</comment>
<file context>
@@ -0,0 +1,365 @@
+ <ct-hstack gap="sm">
+ <icon>question</icon>
+ <button>hello</button>
+ <ct-spacer />
+ <button>hello</button>
+ </ct-hstack>
</file context>
| - gap: `ct-icon` (and `ct-label` has an optional in-built icon) | ||
| - gap: icon set? | ||
|
|
||
| - visual: `ct-kdb`, `ct-separator`, `ct-table`, `ct-tool-call` |
Contributor
There was a problem hiding this comment.
The keyboard component is named ct-kbd, not ct-kdb; please fix the typo so the docs reference the real element.
Prompt for AI agents
Address the following comment on tutorials/common_ui.md at line 310:
<comment>The keyboard component is named `ct-kbd`, not `ct-kdb`; please fix the typo so the docs reference the real element.</comment>
<file context>
@@ -0,0 +1,365 @@
+- gap: `ct-icon` (and `ct-label` has an optional in-built icon)
+ - gap: icon set?
+
+- visual: `ct-kdb`, `ct-separator`, `ct-table`, `ct-tool-call`
+ - gap: `ct-img` or `ct-media`
+
</file context>
Suggested change
| - visual: `ct-kdb`, `ct-separator`, `ct-table`, `ct-tool-call` | |
| - visual: `ct-kbd`, `ct-separator`, `ct-table`, `ct-tool-call` |
|
|
||
| ```{code-block} html | ||
| <ct-screen> | ||
| <ct-toolbar slot="top"> |
Contributor
There was a problem hiding this comment.
ct-screen only exposes header and footer named slots, so using slot="top" here prevents the toolbar from rendering. Please use the header slot instead.
Prompt for AI agents
Address the following comment on tutorials/common_ui.md at line 266:
<comment>`ct-screen` only exposes `header` and `footer` named slots, so using `slot="top"` here prevents the toolbar from rendering. Please use the `header` slot instead.</comment>
<file context>
@@ -0,0 +1,365 @@
+
+```{code-block} html
+<ct-screen>
+ <ct-toolbar slot="top">
+ <ct-button>hello</ct-button>
+ </ct-toolbar>
</file context>
Suggested change
| <ct-toolbar slot="top"> | |
| <ct-toolbar slot="header"> |
| ```{code-block} typescript | ||
| export default recipe("state test", () => { | ||
| const characterName = cell<string>(""); | ||
| } |
Contributor
There was a problem hiding this comment.
Add the missing ); so the second code sample remains valid TypeScript; otherwise readers copying it will hit a syntax error.
Prompt for AI agents
Address the following comment on tutorials/state.md at line 27:
<comment>Add the missing `);` so the second code sample remains valid TypeScript; otherwise readers copying it will hit a syntax error.</comment>
<file context>
@@ -0,0 +1,230 @@
+```{code-block} typescript
+export default recipe("state test", () => {
+ const characterName = cell<string>("");
+}
+```
+Here, we have created a cell with a type argument of `string`,
</file context>
Suggested change
| } | |
| }); |
| title: Guide to the Common Tools Runtime | ||
| description: Documentation and tutorials for the Common Tools runtime and framework | ||
| keywords: [commontools, runtime, tutorials, documentation] | ||
| github: https://github.com/jakedahn/common/tree/main/labs |
Contributor
There was a problem hiding this comment.
The project.github link targets jakedahn/common, so the “Edit on GitHub” links generated by MyST will go to the wrong repository. Update this to the current commontoolsinc/labs/tutorials path so docs readers aren’t sent to the wrong repo.
Prompt for AI agents
Address the following comment on tutorials/myst.yml at line 8:
<comment>The project.github link targets jakedahn/common, so the “Edit on GitHub” links generated by MyST will go to the wrong repository. Update this to the current commontoolsinc/labs/tutorials path so docs readers aren’t sent to the wrong repo.</comment>
<file context>
@@ -0,0 +1,28 @@
+ title: Guide to the Common Tools Runtime
+ description: Documentation and tutorials for the Common Tools runtime and framework
+ keywords: [commontools, runtime, tutorials, documentation]
+ github: https://github.com/jakedahn/common/tree/main/labs
+ # To autogenerate a Table of Contents, run "myst init --write-toc"
+ toc:
</file context>
Suggested change
| github: https://github.com/jakedahn/common/tree/main/labs | |
| github: https://github.com/commontoolsinc/labs/tree/main/tutorials |
| @@ -0,0 +1,14 @@ | |||
| # Tutorials | |||
|
|
|||
| Install MyST Markdown tooling by following https://mystmd.org/guide/installing. Once set up, run `myst` in this directory to serve the tutorial pages locally. | |||
Contributor
There was a problem hiding this comment.
Running myst without a subcommand just prints the CLI help, so this quickstart won’t launch the local docs server. Replace it with myst start to match the working command used in the project scripts.
Prompt for AI agents
Address the following comment on tutorials/README.md at line 3:
<comment>Running `myst` without a subcommand just prints the CLI help, so this quickstart won’t launch the local docs server. Replace it with `myst start` to match the working command used in the project scripts.</comment>
<file context>
@@ -0,0 +1,14 @@
+# Tutorials
+
+Install MyST Markdown tooling by following https://mystmd.org/guide/installing. Once set up, run `myst` in this directory to serve the tutorial pages locally.
+
+## npm quickstart
</file context>
Suggested change
| Install MyST Markdown tooling by following https://mystmd.org/guide/installing. Once set up, run `myst` in this directory to serve the tutorial pages locally. | |
| Install MyST Markdown tooling by following https://mystmd.org/guide/installing. Once set up, run `myst start` in this directory to serve the tutorial pages locally. |
| "version": "0.0.1", | ||
| "scripts": { | ||
| "build": "myst build --html", | ||
| "clean": "myst clean --cache || true", |
Contributor
There was a problem hiding this comment.
The clean script uses || true to swallow errors, but true is a POSIX utility and is missing on Windows, causing npm run clean to fail there. Replace the fallback with a cross-platform no-op such as node -e "process.exit(0)".
Prompt for AI agents
Address the following comment on tutorials/package.json at line 7:
<comment>The `clean` script uses `|| true` to swallow errors, but `true` is a POSIX utility and is missing on Windows, causing `npm run clean` to fail there. Replace the fallback with a cross-platform no-op such as `node -e "process.exit(0)"`.</comment>
<file context>
@@ -0,0 +1,13 @@
+ "version": "0.0.1",
+ "scripts": {
+ "build": "myst build --html",
+ "clean": "myst clean --cache || true",
+ "dev": "myst start"
+ },
</file context>
Suggested change
| "clean": "myst clean --cache || true", | |
| "clean": "myst clean --cache || node -e \"process.exit(0)\"", |
| ## GitHub Actions Deployment | ||
|
|
||
| The documentation is automatically built and deployed via GitHub Actions when: | ||
| - Changes are pushed to the `main` branch |
Contributor
There was a problem hiding this comment.
This bullet implies that only pushes to main deploy the docs, but the workflow also deploys when migrate-tutorials is pushed. Please update the documentation to mention both branches.
Prompt for AI agents
Address the following comment on tutorials/README_BUILD.md at line 12:
<comment>This bullet implies that only pushes to main deploy the docs, but the workflow also deploys when migrate-tutorials is pushed. Please update the documentation to mention both branches.</comment>
<file context>
@@ -0,0 +1,53 @@
+## GitHub Actions Deployment
+
+The documentation is automatically built and deployed via GitHub Actions when:
+- Changes are pushed to the `main` branch
+- The changes affect files in the `tutorials/` directory
+- Or when manually triggered via workflow dispatch
</file context>
Suggested change
| - Changes are pushed to the `main` branch | |
| - Changes are pushed to the `main` or `migrate-tutorials` branches |
| - name: Copy CNAME to build directory | ||
| run: cp tutorials/CNAME tutorials/_build/html/CNAME | ||
|
|
||
| - name: Upload artifact |
Contributor
There was a problem hiding this comment.
Insert an actions/configure-pages step before uploading the artifact; otherwise actions/upload-pages-artifact will fail because Pages was never configured.
Prompt for AI agents
Address the following comment on .github/workflows/deploy-docs.yml at line 54:
<comment>Insert an actions/configure-pages step before uploading the artifact; otherwise actions/upload-pages-artifact will fail because Pages was never configured.</comment>
<file context>
@@ -0,0 +1,70 @@
+ - name: Copy CNAME to build directory
+ run: cp tutorials/CNAME tutorials/_build/html/CNAME
+
+ - name: Upload artifact
+ uses: actions/upload-pages-artifact@v3
+ with:
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Migrates the tutorials into the repo and sets up automated MyST docs builds and deployment to docs.commontools.dev. Adds PR validation builds and publishes on pushes to main and migrate-tutorials.
New Features
Migration