Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue with the documentation autoupdate script by correcting the target destination paths for generated documentation. The change ensures that updated documentation is placed into the appropriate versioned subdirectory, aligning with the intended documentation structure. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
📝 WalkthroughWalkthroughMakefile variables updated (expanded Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Code Review
The pull request correctly updates the destination directory paths in the Makefile to include the v1 version segment for documentation. This aligns with the versioned documentation structure. I've suggested a small refactoring to use a variable for the versioned path, which will make future updates (e.g., to v2) easier to manage.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
7fb949c to
6ff60ac
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
content/en/docs/v1/guides/platform-stack/_index.md (1)
212-224:⚠️ Potential issue | 🟡 MinorRename the managed-app link label to
MariaDB.The section is now explicitly about MariaDB, and the link target already points to the MariaDB page, but Line 224 still renders the label as
MySQL. That reads like a stale reference in the published docs.Suggested doc fix
-- Managed application reference: [MySQL]({{% ref "/docs/v1/applications/mariadb" %}}) +- Managed application reference: [MariaDB]({{% ref "/docs/v1/applications/mariadb" %}})🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/v1/guides/platform-stack/_index.md` around lines 212 - 224, Update the markdown link label so it matches the section topic: replace the link "[MySQL]({{% ref "/docs/v1/applications/mariadb" %}})" with "[MariaDB]({{% ref "/docs/v1/applications/mariadb" %}})"; this ensures the label shown (currently "MySQL") is renamed to "MariaDB" in the MariaDB (MySQL fork) section.Makefile (1)
32-33:⚠️ Potential issue | 🟠 Major
update-apistill writes to the old non-v1docs path.The rest of this file was migrated to
content/en/docs/v1/..., but Line 33 still updatescontent/en/docs/cozystack-api/api.json. That means the API refresh target is now out of sync with the v1 docs tree and will keep updating the stale location instead of the page that is actually rendered.Suggested fix
update-api: - kubectl get --raw '/openapi/v3/apis/apps.cozystack.io/v1alpha1' > content/en/docs/cozystack-api/api.json + kubectl get --raw '/openapi/v3/apis/apps.cozystack.io/v1alpha1' > content/en/docs/v1/cozystack-api/api.json🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Makefile` around lines 32 - 33, The update-api Makefile target is still writing the OpenAPI JSON to the old path "content/en/docs/cozystack-api/api.json"; change the output path in the update-api recipe to the new v1 docs location (e.g. "content/en/docs/v1/cozystack-api/api.json") so the API refresh updates the rendered v1 docs tree instead of the stale file; update the target that starts with "update-api:" and replace the old output filename accordingly.
🧹 Nitpick comments (2)
content/en/docs/v1/applications/qdrant.md (1)
1-5: Add the standard application aliases to the new page.Sibling app pages in this section expose both
/docs/reference/applications/<app>and/docs/v1/reference/applications/<app>aliases. Leaving them off here makesqdrant.mdfollow a different routing pattern than the rest of the application docs.Suggested fix
--- title: "Managed Qdrant Service" linkTitle: "Qdrant" weight: 50 +aliases: + - /docs/reference/applications/qdrant + - /docs/v1/reference/applications/qdrant ---🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/v1/applications/qdrant.md` around lines 1 - 5, The page frontmatter for qdrant.md is missing the standard aliases used by sibling app pages; update the YAML frontmatter (the block containing title and linkTitle) to add an aliases entry that includes both "/docs/reference/applications/qdrant" and "/docs/v1/reference/applications/qdrant" so the page exposes the same routes as other application docs while leaving title, linkTitle, and weight unchanged.content/en/docs/v1/virtualization/_index.md (1)
15-18: Remove the orphanedvirtual-machinesource as part of this cleanup.This section now says there are only two virtualization packages, but
content/en/docs/v1/virtualization/_include/virtual-machine.md:1-9still exists with its own front matter and aliases. Keeping the stale include around makes the source tree contradict the index and makes it easy to reintroduce the removed page later.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@content/en/docs/v1/virtualization/_index.md` around lines 15 - 18, Orphaned include "virtual-machine.md" remains despite the index listing only two packages; remove the stale include file (virtual-machine.md) from the virtualization includes or strip its front matter/aliases so it no longer registers as a page, and then confirm there are no remaining references or aliases to "virtual-machine" in the virtualization index or other docs (check _index.md and any include/import references) so the source tree matches the index.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@content/en/docs/v1/applications/harbor.md`:
- Line 14: Replace the sentence "Harbor is an open source trusted cloud native
registry project that stores, signs, and scans content." with a tightened
wording that hyphenates the compound adjectives: use "open-source" and
"cloud-native" (e.g., "Harbor is an open-source, cloud-native trusted registry
project that stores, signs, and scans content.").
In `@content/en/docs/v1/applications/mariadb.md`:
- Line 119: The docs currently show backup.s3Bucket but the example uses an
endpoint-plus-path; update the documentation so backup.s3Bucket and its example
consistently indicate the expected value format: either (A) accept a plain
bucket name (e.g., change the example to `mariadb-backups` and keep the
description "S3 bucket used for storing backups"), or (B) accept a full S3
endpoint/path (e.g., change the description to "S3 endpoint and bucket/path used
for storing backups" and set the example to `s3.example.org/mariadb-backups`);
pick one and make the description and the example for backup.s3Bucket match.
- Line 32: Replace the misspelled placeholder in the kubectl edit example:
change the command snippet "kubectl edit mariadb <instnace>" to use the correct
placeholder "kubectl edit mariadb <instance>" so the docs show a valid, copyable
example; update the snippet in the mariadb.md content where the command appears.
- Around line 71-75: Fix the typos in this troubleshooting block: change
"inmplemented" to "implemented" in the sentence mentioning mariadbbackup and
mariadb-operator, and correct "indicies" (and any other occurrences like
"indecies") to "indices" in the "**Corrupted indicies**" heading and body text;
also consider renaming the heading to "**Corrupted indices**" for consistency
with the corrected spelling and run a quick spell-check on the surrounding lines
(mentions of "mariadbbackup" and "mariadb-operator" should remain unchanged).
In `@content/en/docs/v1/applications/tenant.md`:
- Around line 23-31: The examples later in the tenant.md page still use prefixed
names like `tenant-u1`/`tenant-u2`, which contradicts the new guidance that
tenant names are simple labels (e.g., `root`, `foo`) and that the `tenant-`
prefix is added internally; update the examples to use the bare-label form
(replace `tenant-u1`/`tenant-u2` with `u1`/`u2` or `root`/`foo`) or
alternatively add a short clarifying line where those examples appear explicitly
stating "internal resources are named `tenant-<label>` but users should provide
the label only (e.g., `u1`, `u2`)." Ensure references in commands, YAML
snippets, and explanatory text consistently use the chosen approach so there is
no implication users must include the `tenant-` prefix.
In `@hack/fill_templates.sh`:
- Around line 76-80: The curl assignment to readme_content currently aborts the
whole script under set -euo pipefail when curl fails; change the readme_content
assignment so curl failures do not trigger an exit (for example by making the
curl command return an empty string or by guarding the call so its non-zero exit
is ignored), then keep the existing if [[ -z "$readme_content" ]] check and
continue; target the readme_content variable and the curl invocation that sets
it so one failed fetch does not stop the whole script.
---
Outside diff comments:
In `@content/en/docs/v1/guides/platform-stack/_index.md`:
- Around line 212-224: Update the markdown link label so it matches the section
topic: replace the link "[MySQL]({{% ref "/docs/v1/applications/mariadb" %}})"
with "[MariaDB]({{% ref "/docs/v1/applications/mariadb" %}})"; this ensures the
label shown (currently "MySQL") is renamed to "MariaDB" in the MariaDB (MySQL
fork) section.
In `@Makefile`:
- Around line 32-33: The update-api Makefile target is still writing the OpenAPI
JSON to the old path "content/en/docs/cozystack-api/api.json"; change the output
path in the update-api recipe to the new v1 docs location (e.g.
"content/en/docs/v1/cozystack-api/api.json") so the API refresh updates the
rendered v1 docs tree instead of the stale file; update the target that starts
with "update-api:" and replace the old output filename accordingly.
---
Nitpick comments:
In `@content/en/docs/v1/applications/qdrant.md`:
- Around line 1-5: The page frontmatter for qdrant.md is missing the standard
aliases used by sibling app pages; update the YAML frontmatter (the block
containing title and linkTitle) to add an aliases entry that includes both
"/docs/reference/applications/qdrant" and
"/docs/v1/reference/applications/qdrant" so the page exposes the same routes as
other application docs while leaving title, linkTitle, and weight unchanged.
In `@content/en/docs/v1/virtualization/_index.md`:
- Around line 15-18: Orphaned include "virtual-machine.md" remains despite the
index listing only two packages; remove the stale include file
(virtual-machine.md) from the virtualization includes or strip its front
matter/aliases so it no longer registers as a page, and then confirm there are
no remaining references or aliases to "virtual-machine" in the virtualization
index or other docs (check _index.md and any include/import references) so the
source tree matches the index.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 087c012b-f1db-438e-811e-b2a42c74f3ba
📒 Files selected for processing (34)
Makefilecontent/en/docs/v1/applications/_include/clickhouse.mdcontent/en/docs/v1/applications/_include/ferretdb.mdcontent/en/docs/v1/applications/_include/foundationdb.mdcontent/en/docs/v1/applications/_include/harbor.mdcontent/en/docs/v1/applications/_include/kafka.mdcontent/en/docs/v1/applications/_include/mariadb.mdcontent/en/docs/v1/applications/_include/mongodb.mdcontent/en/docs/v1/applications/_include/openbao.mdcontent/en/docs/v1/applications/_include/postgres.mdcontent/en/docs/v1/applications/_include/qdrant.mdcontent/en/docs/v1/applications/_include/rabbitmq.mdcontent/en/docs/v1/applications/_include/redis.mdcontent/en/docs/v1/applications/_include/tenant.mdcontent/en/docs/v1/applications/clickhouse.mdcontent/en/docs/v1/applications/ferretdb.mdcontent/en/docs/v1/applications/foundationdb.mdcontent/en/docs/v1/applications/harbor.mdcontent/en/docs/v1/applications/kafka.mdcontent/en/docs/v1/applications/mariadb.mdcontent/en/docs/v1/applications/mongodb.mdcontent/en/docs/v1/applications/nats.mdcontent/en/docs/v1/applications/openbao.mdcontent/en/docs/v1/applications/postgres.mdcontent/en/docs/v1/applications/qdrant.mdcontent/en/docs/v1/applications/rabbitmq.mdcontent/en/docs/v1/applications/redis.mdcontent/en/docs/v1/applications/tenant.mdcontent/en/docs/v1/guides/platform-stack/_index.mdcontent/en/docs/v1/operations/services/_include/monitoring.mdcontent/en/docs/v1/virtualization/_index.mdcontent/en/docs/v1/virtualization/proxmox-migration.mdcontent/en/docs/v1/virtualization/virtual-machine.mdhack/fill_templates.sh
💤 Files with no reviewable changes (3)
- content/en/docs/v1/applications/_include/ferretdb.md
- content/en/docs/v1/applications/ferretdb.md
- content/en/docs/v1/virtualization/virtual-machine.md
✅ Files skipped from review due to trivial changes (8)
- content/en/docs/v1/applications/openbao.md
- content/en/docs/v1/applications/clickhouse.md
- content/en/docs/v1/operations/services/_include/monitoring.md
- content/en/docs/v1/applications/_include/harbor.md
- content/en/docs/v1/applications/_include/kafka.md
- content/en/docs/v1/applications/foundationdb.md
- content/en/docs/v1/applications/_include/foundationdb.md
- content/en/docs/v1/applications/redis.md
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hack/fill_templates.sh`:
- Around line 76-80: The curl call that populates readme_content must detect
HTTP errors: restore the -f flag and stop swallowing failures so HTTP 4xx/5xx
return non-zero. Replace the current invocation (the one producing
readme_content) with curl -sSLf "$readme_url" and remove the trailing "|| true"
so that readme_content will be empty on fetch errors and the existing if [[ -z
"$readme_content" ]] check will handle missing READMEs; look for the
readme_content assignment in fill_templates.sh to update it.
hack/fill_templates.sh
Outdated
| readme_content=$(curl -sSL "$readme_url") || true | ||
| if [[ -z "$readme_content" ]]; then | ||
| echo "⚠️ Failed to fetch README for $app" >&2 | ||
| continue | ||
| fi |
There was a problem hiding this comment.
Missing -f flag causes HTTP errors to go undetected.
The -f flag was removed from curl. Without it, curl returns exit code 0 even on HTTP 4xx/5xx errors (e.g., 404 for a non-existent README). The || true only suppresses network-level failures, not HTTP errors.
For a missing README, curl will return GitHub's HTML error page, pass the empty check, and the script will extract garbage HTML as the "title".
Suggested fix: restore -f flag
- readme_content=$(curl -sSL "$readme_url") || true
+ readme_content=$(curl -fsSL "$readme_url") || true📝 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.
| readme_content=$(curl -sSL "$readme_url") || true | |
| if [[ -z "$readme_content" ]]; then | |
| echo "⚠️ Failed to fetch README for $app" >&2 | |
| continue | |
| fi | |
| readme_content=$(curl -fsSL "$readme_url") || true | |
| if [[ -z "$readme_content" ]]; then | |
| echo "⚠️ Failed to fetch README for $app" >&2 | |
| continue | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/fill_templates.sh` around lines 76 - 80, The curl call that populates
readme_content must detect HTTP errors: restore the -f flag and stop swallowing
failures so HTTP 4xx/5xx return non-zero. Replace the current invocation (the
one producing readme_content) with curl -sSLf "$readme_url" and remove the
trailing "|| true" so that readme_content will be empty on fetch errors and the
existing if [[ -z "$readme_content" ]] check will handle missing READMEs; look
for the readme_content assignment in fill_templates.sh to update it.
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
a676621 to
5ee8917
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
hack/fill_templates.sh (1)
71-102: Consider tracking partial failures for caller visibility.Currently, the script exits with code 0 even if some apps fail to fetch. The Makefile targets invoking this script won't detect partial failures. For CI/observability, you may want to track failures and exit non-zero when some apps failed.
💡 Optional: Track and report partial failures
+failed_apps=() + for app in "${APPS[@]}"; do readme_url="${RAW_BASE_URL}/${app}/README.md" echo "Processing $app..." # Try to fetch the first line (title) of the README readme_content=$(curl -fsSL "$readme_url") || true if [[ -z "$readme_content" ]]; then echo "⚠️ Failed to fetch README for $app" >&2 + failed_apps+=("$app") continue fi first_line=$(echo "$readme_content" | head -n 1) if [[ -z "$first_line" ]]; then echo "⚠️ README for $app has no content" >&2 + failed_apps+=("$app") continue fi # ... rest of loop done + +if [[ ${`#failed_apps`[@]} -gt 0 ]]; then + echo "⚠️ Failed to process: ${failed_apps[*]}" >&2 + exit 1 +fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hack/fill_templates.sh` around lines 71 - 102, The script currently continues on per-app failures inside the for app in "${APPS[@]}" loop (checks around readme_content and first_line) but still exits 0; add a failure counter (e.g., failed=0) incremented whenever you hit a curl or README-empty branch before the continue, and after the loop check if failed > 0 then print a summary message and exit with a non-zero status (exit 1); update the echo messages to include which app failed so the summary is meaningful while leaving the successful template generation (cat -> "$template_file") unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@hack/fill_templates.sh`:
- Around line 71-102: The script currently continues on per-app failures inside
the for app in "${APPS[@]}" loop (checks around readme_content and first_line)
but still exits 0; add a failure counter (e.g., failed=0) incremented whenever
you hit a curl or README-empty branch before the continue, and after the loop
check if failed > 0 then print a summary message and exit with a non-zero status
(exit 1); update the echo messages to include which app failed so the summary is
meaningful while leaving the successful template generation (cat ->
"$template_file") unchanged.
Summary by CodeRabbit