docs(helm): Add Helm chart downloading steps to the quick start guide; Rename the quick start "Overview" page as "First steps" for clarity.#1887
Conversation
WalkthroughUpdated quick-start docs to download and extract the Helm chart tarball into Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/src/user-docs/quick-start/index.md (1)
54-78: Add Git to Kubernetes system requirements.The updated Kubernetes quick-start workflow now requires users to clone the repository (as seen in
clp-json.mdlines 101-102 andclp-text.mdlines 111-112), but Git is not listed in the system requirements.📝 Suggested addition to system requirements
Add Git to the requirements list and verification commands:
[`kind`][kind] (Kubernetes in Docker) runs a Kubernetes cluster inside Docker containers, making it ideal for local Kubernetes testing and development. * [Docker][Docker] (required for `kind`) * `containerd.io` >= 1.7.18 * `docker-ce` >= 27.0.3 * `docker-ce-cli` >= 27.0.3 * [`kind`][kind] >= 0.23 * [`kubectl`][kubectl] >= 1.30 * [Helm][Helm] >= 4.0 +* [Git][Git] To check whether the required tools are installed on your system, run: ```bash containerd --version docker version --format '{{.Server.Version}}' kind version kubectl version --client --output=yaml | grep gitVersion helm version --short +git --versionThen add the Git reference link at the bottom of the file alongside the other reference links. </details> </blockquote></details> </blockquote></details>🤖 Fix all issues with AI agents
In `@docs/src/user-docs/quick-start/clp-json.md`: - Around line 101-104: The cloning instruction is ambiguous; update the text around the git clone and cd lines so it is explicit: either remove "(if you haven't already)" and state a single step that users should run to clone the repo (the line starting with "git clone --branch DOCS_VAR_CLP_GIT_REF ..."), or rewrite to handle both cases by adding a short conditional note that if the repo is already cloned, skip the git clone step and otherwise run it, and ensure the following "cd clp/tools/deployment/package-helm" step clearly references the cloned repo directory (i.e., instruct users to cd into the cloned repository root first if they ran the clone). Ensure the updated content keeps the existing git clone and cd commands but clarifies the user flow.♻️ Duplicate comments (1)
docs/src/user-docs/quick-start/clp-text.md (1)
111-114: Clarify the repository cloning instruction.The comment "(if you haven't already)" is confusing since this is the first mention of cloning in the quick-start workflow. Additionally, the conditional phrasing doesn't align well with the subsequent
cdcommand, which assumes the repository was cloned to the default location.✏️ Suggested clarification
-# Clone the repository (if you haven't already) +# Clone the repository and navigate to the Helm chart directory git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git cd clp/tools/deployment/package-helmAlternatively, if you want to accommodate users who may have already cloned:
-# Clone the repository (if you haven't already) -git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git -cd clp/tools/deployment/package-helm +# Clone the repository +git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git + +# Navigate to the Helm chart directory +cd clp/tools/deployment/package-helm
| # Clone the repository (if you haven't already) | ||
| git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git | ||
| cd clp/tools/deployment/package-helm | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify the repository cloning instruction.
The comment "(if you haven't already)" is confusing since this is the first mention of cloning in the quick-start workflow. Additionally, the conditional phrasing doesn't align well with the subsequent cd command, which assumes the repository was cloned to the default location.
✏️ Suggested clarification
-# Clone the repository (if you haven't already)
+# Clone the repository and navigate to the Helm chart directory
git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git
cd clp/tools/deployment/package-helmAlternatively, if you want to accommodate users who may have already cloned:
-# Clone the repository (if you haven't already)
-git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git
-cd clp/tools/deployment/package-helm
+# Clone the repository
+git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git
+
+# Navigate to the Helm chart directory
+cd clp/tools/deployment/package-helm📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Clone the repository (if you haven't already) | |
| git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git | |
| cd clp/tools/deployment/package-helm | |
| # Clone the repository and navigate to the Helm chart directory | |
| git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git | |
| cd clp/tools/deployment/package-helm | |
🤖 Prompt for AI Agents
In `@docs/src/user-docs/quick-start/clp-json.md` around lines 101 - 104, The
cloning instruction is ambiguous; update the text around the git clone and cd
lines so it is explicit: either remove "(if you haven't already)" and state a
single step that users should run to clone the repo (the line starting with "git
clone --branch DOCS_VAR_CLP_GIT_REF ..."), or rewrite to handle both cases by
adding a short conditional note that if the repo is already cloned, skip the git
clone step and otherwise run it, and ensure the following "cd
clp/tools/deployment/package-helm" step clearly references the cloned repo
directory (i.e., instruct users to cd into the cloned repository root first if
they ran the clone). Ensure the updated content keeps the existing git clone and
cd commands but clarifies the user flow.
kirkrodrigues
left a comment
There was a problem hiding this comment.
Sorry, just realized something.
| ```bash | ||
| cd tools/deployment/package-helm | ||
| # Clone the repository (if you haven't already) | ||
| git clone --branch DOCS_VAR_CLP_GIT_REF https://github.com/y-scope/clp.git |
There was a problem hiding this comment.
If we read the guide from the top, then we're going to create the data directories in $HOME/clp. Then we're going to clone the repository into ./clp. If the user hasn't changed directories from ~, then this will also be where the data directories already exist, right? Should we perhaps make the user create the data into something like $HOME/clp-data? That would also make it more obvious that it's data and shouldn't be blindly deleted.
… download instructions
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/src/user-docs/quick-start/clp-text.md`:
- Around line 111-117: The curl->tar pipeline (the line starting with "curl
--silent \") can hide HTTP errors and lead to confusing tar errors; update the
pipeline to enable failure visibility by adding curl flags --fail and
--show-error (so curl exits non‑zero on HTTP errors and prints messages) and
ensure the shell exits on pipeline failures (enable set -o pipefail or set -euo
pipefail before the commands) or explicitly check curl's exit status before
piping to tar; keep the existing tar invocation ("tar --extract --gzip
--strip-components=4 --directory \"$HOME/clp-package-helm\"") but only run it
after confirming the download succeeded.
| curl --silent \ | ||
| --location https://github.com/y-scope/clp/archive/refs/heads/DOCS_VAR_CLP_GIT_REF.tar.gz \ |
There was a problem hiding this comment.
| curl --silent \ | |
| --location https://github.com/y-scope/clp/archive/refs/heads/DOCS_VAR_CLP_GIT_REF.tar.gz \ | |
| curl \ | |
| --silent \ | |
| --location https://github.com/y-scope/clp/archive/refs/heads/DOCS_VAR_CLP_GIT_REF.tar.gz \ |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/src/user-docs/quick-start/clp-json.md`:
- Around line 101-112: The document includes verbose procedural extraction steps
(the curl/tar sequence that creates "$HOME/clp-package-helm" and extracts
"clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm"); trim this by
replacing the full command block with a short conceptual summary (e.g.,
"download and extract the Helm chart into a local directory") and add a clear
link to the detailed step‑by‑step video or external guide for users who need
exact commands, keeping only one-line notes about prerequisites (like ensuring
curl/tar are installed) and the target path.
In `@docs/src/user-docs/quick-start/clp-text.md`:
- Around line 111-122: The detailed step-by-step extraction block (the
mkdir/curl | tar pipeline that creates "$HOME/clp-package-helm" and extracts
"clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm") should be shortened to
a one-line conceptual instruction and a link to the procedural video/resource;
move the full command sequence into a linked appendix or external gist/how-to
page and replace the current lines with a brief summary like "Download and
extract the Helm chart (see video/appendix for commands)". Ensure the summary
references the target folder name ("$HOME/clp-package-helm") and the package
path ("clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm") so readers can
find the full commands in the linked resource.
♻️ Duplicate comments (2)
docs/src/user-docs/quick-start/clp-text.md (1)
111-122: Handle download failures explicitly in the curl→tar pipeline.This mirrors a previously raised concern: without
--fail --show-error(or equivalent) and pipefail behaviour, HTTP failures are easy to miss and tar errors become confusing. Please align with the earlier review comment for this block.docs/src/user-docs/quick-start/clp-json.md (1)
101-112: Handle download failures explicitly in the curl→tar pipeline.Same issue as previously noted:
curl --silentin a pipe can mask HTTP errors; add explicit failure visibility/pipefail per prior review guidance.
| # Download and extract the Helm chart from the CLP repository. | ||
| mkdir -p "$HOME/clp-package-helm" | ||
| curl \ | ||
| --silent \ | ||
| --location https://github.com/y-scope/clp/archive/refs/heads/DOCS_VAR_CLP_GIT_REF.tar.gz \ | ||
| | tar \ | ||
| --extract \ | ||
| --gzip \ | ||
| --strip-components=4 \ | ||
| --directory "$HOME/clp-package-helm" \ | ||
| "clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm" | ||
| cd "$HOME/clp-package-helm" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider moving detailed extract steps to a linked video or a brief summary.
This level of procedural detail conflicts with the team preference to keep docs conceptual and rely on video for step-by-step actions. Consider trimming and linking out. Based on learnings, ...
🤖 Prompt for AI Agents
In `@docs/src/user-docs/quick-start/clp-json.md` around lines 101 - 112, The
document includes verbose procedural extraction steps (the curl/tar sequence
that creates "$HOME/clp-package-helm" and extracts
"clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm"); trim this by
replacing the full command block with a short conceptual summary (e.g.,
"download and extract the Helm chart into a local directory") and add a clear
link to the detailed step‑by‑step video or external guide for users who need
exact commands, keeping only one-line notes about prerequisites (like ensuring
curl/tar are installed) and the target path.
| # Download and extract the Helm chart from the CLP repository. | ||
| mkdir -p "$HOME/clp-package-helm" | ||
| curl \ | ||
| --silent \ | ||
| --location https://github.com/y-scope/clp/archive/refs/heads/DOCS_VAR_CLP_GIT_REF.tar.gz \ | ||
| | tar \ | ||
| --extract \ | ||
| --gzip \ | ||
| --strip-components=4 \ | ||
| --directory "$HOME/clp-package-helm" \ | ||
| "clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm" | ||
| cd "$HOME/clp-package-helm" |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider moving detailed extract steps to a linked video or a brief summary.
These lines introduce step-by-step extraction details. Team preference is to keep written docs conceptual and use video for procedural walkthroughs; consider shortening this block and linking to a video/resource. Based on learnings, ...
🤖 Prompt for AI Agents
In `@docs/src/user-docs/quick-start/clp-text.md` around lines 111 - 122, The
detailed step-by-step extraction block (the mkdir/curl | tar pipeline that
creates "$HOME/clp-package-helm" and extracts
"clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm") should be shortened to
a one-line conceptual instruction and a link to the procedural video/resource;
move the full command sequence into a linked appendix or external gist/how-to
page and replace the current lines with a brief summary like "Download and
extract the Helm chart (see video/appendix for commands)". Ensure the summary
references the target folder name ("$HOME/clp-package-helm") and the package
path ("clp-DOCS_VAR_CLP_GIT_REF/tools/deployment/package-helm") so readers can
find the full commands in the linked resource.
…; Rename the quick start "Overview" page as "First steps" for clarity. (y-scope#1887)
Description
As the title says, this PR aims to narrow the gap for readers unfamiliar with the project.
Checklist
breaking change.
Validation performed
task docs:serveand confirmed in the browser that the docs are modified as expected.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.