docs: add run script documentation#2892
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDocs updated to add a new on-demand, restartable Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
No issues found across 1 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
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/docs/content/docs/setup-teardown-scripts.mdx (1)
102-109:⚠️ Potential issue | 🟠 MajorDocumentation contradicts actual implementation behavior.
Line 109 states "No merging occurs between levels—the first config found is used entirely," but the implementation in
setup.tsshows key-level merging with inheritance. Per the code comment: "Higher-priority configs override only the keys they explicitly define. Missing keys inherit from lower-priority sources."This means if a worktree config defines only
setup, therunandteardownfields would still be inherited from the main repo config—not replaced entirely as documented here.Suggested fix
### Priority Order -Config is resolved in this order (first found wins, for setup, teardown, and run): +Config is resolved in this order (higher priority overrides lower, per key): 1. `~/.superset/projects/<project-id>/config.json` — user override 2. `<worktree>/.superset/config.json` — worktree-specific 3. `<repo>/.superset/config.json` — project default -No merging occurs between levels—the first config found is used entirely. +Higher-priority configs override only the keys they explicitly define. Missing keys inherit from lower-priority sources. For example, if the worktree config defines only `setup`, the `teardown` and `run` from the main repo config are still used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/content/docs/setup-teardown-scripts.mdx` around lines 102 - 109, The docs incorrectly state "No merging occurs..." but the implementation in setup.ts performs key-level merging/inheritance: higher-priority configs override only the keys they define and missing keys (e.g., run, teardown) inherit from lower-priority sources; update the section to describe this behavior (mentioning setup.ts and the keys 'setup', 'run', 'teardown') and provide the correct priority/resolution wording explaining key-level inheritance rather than whole-file replacement.
🧹 Nitpick comments (1)
apps/docs/content/docs/setup-teardown-scripts.mdx (1)
130-146: Consider addingrunexamples to local config section.The implementation supports
before/aftersyntax forruncommands as well (seemergeConfigsinsetup.tswhich iterates over["setup", "teardown", "run"]), but the examples only showsetupandteardown. Adding arunexample would make the documentation more complete.Example addition
{ "setup": { "before": ["echo 'running pre-setup'"], "after": ["./.superset/my-post-setup.sh"] }, "teardown": { "after": ["./.superset/my-cleanup.sh"] - } + }, + "run": { + "before": ["echo 'starting services'"] + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/docs/content/docs/setup-teardown-scripts.mdx` around lines 130 - 146, Add an example showing the `run` key with `before`/`after` arrays to mirror the existing `setup`/`teardown` example because `mergeConfigs` processes `["setup","teardown","run"]`; update the docs example to include a `"run": { "before": [...], "after": [...] }` block using the same style and note that unspecified keys pass through unchanged so readers see `before` → committed run scripts → `after` behavior for `run` as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/docs/content/docs/setup-teardown-scripts.mdx`:
- Around line 102-109: The docs incorrectly state "No merging occurs..." but the
implementation in setup.ts performs key-level merging/inheritance:
higher-priority configs override only the keys they define and missing keys
(e.g., run, teardown) inherit from lower-priority sources; update the section to
describe this behavior (mentioning setup.ts and the keys 'setup', 'run',
'teardown') and provide the correct priority/resolution wording explaining
key-level inheritance rather than whole-file replacement.
---
Nitpick comments:
In `@apps/docs/content/docs/setup-teardown-scripts.mdx`:
- Around line 130-146: Add an example showing the `run` key with
`before`/`after` arrays to mirror the existing `setup`/`teardown` example
because `mergeConfigs` processes `["setup","teardown","run"]`; update the docs
example to include a `"run": { "before": [...], "after": [...] }` block using
the same style and note that unspecified keys pass through unchanged so readers
see `before` → committed run scripts → `after` behavior for `run` as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e73696d0-9472-4b14-a2ee-2ba8164e38cd
📒 Files selected for processing (1)
apps/docs/content/docs/setup-teardown-scripts.mdx
The `run` field in `.superset/config.json` was undocumented. This adds a section explaining how run scripts work (on-demand via Run button, restartable, dedicated pane) and how they differ from setup/teardown. Also adds the missing `SUPERSET_WORKSPACE_PATH` environment variable. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
vitorvavolizza
force-pushed
the
docs/run-script-config
branch
from
41539d7 to
dd66a16
Compare
|
You beat me to it! Looks good 👍 |
vitorvavolizza
had a problem deploying
to
preview
GitHub Actions
Failure
|
lgtm |
3 participants
Summary
runfield in.superset/config.json, which was previously undocumentedSUPERSET_WORKSPACE_PATHenvironment variable to the reference tableTest plan
/docs/setup-teardown-scripts🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
runcommand support: Adds an on-demand, restartablerunaction triggered via a Run button in a dedicated terminal pane, separate from automaticsetup/teardown.setupandteardownrun automatically in sequence on workspace create/delete.SUPERSET_WORKSPACE_PATH, includesrunin config priority order, full config examples, and tips to userunfor dev servers.