-
Notifications
You must be signed in to change notification settings - Fork 19
fix auto-generation of documentation #391
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
Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoved a Monitoring hub page, moved inline monitoring content to includes, added parameter reference files, adjusted navigation weights, and updated the build script to special-case the monitoring parameters path. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
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 |
Summary of ChangesHello @IvanHunters, 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 resolves an issue related to the auto-generation of documentation. It involves relocating the 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. 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request aims to fix the auto-generation of documentation by moving content and adding parameter tables. However, it introduces a critical structural issue by creating content/en/docs/operations/services/monitoring.md, which conflicts with the existing content/en/docs/operations/services/monitoring/ directory and will break the Hugo site build. The content should instead be merged into content/en/docs/operations/services/monitoring/_index.md. I've also pointed out a broken metadata link resulting from the file move, and a couple of areas in the generated documentation tables where descriptions and values could be clearer for better user understanding.
| --- | ||
| title: "Monitoring Hub Reference" | ||
| linkTitle: "Monitoring" | ||
| --- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new file monitoring.md is created in the content/en/docs/operations/services/ directory, but a directory named monitoring/ already exists at the same level. In Hugo, a file (monitoring.md) and a directory (monitoring/) with the same base name cannot coexist as siblings. This will break the website build.
The content of this file should be merged into content/en/docs/operations/services/monitoring/_index.md, which serves as the main page for the monitoring section. This will consolidate the monitoring documentation correctly without causing build failures.
|
|
||
| <!-- | ||
| Autogenerated content. Don't edit this file directly; edit sources instead. | ||
| metadata: https://github.com/cozystack/website/blob/main/content/en/docs/operations/services/_include/monitoring.md |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The metadata link points to .../_include/monitoring.md, which is deleted in this pull request, creating a broken link. This should be updated. Given the structural issue with this file's location, the link should ultimately point to .../monitoring/_index.md once the content is moved there.
| metadata: https://github.com/cozystack/website/blob/main/content/en/docs/operations/services/_include/monitoring.md | |
| metadata: https://github.com/cozystack/website/blob/main/content/en/docs/operations/services/monitoring/_index.md |
| | ---------------------------------- | ---------------------------------------- | ---------- | ------------ | | ||
| | `logsStorages` | Configuration of logs storage instances. | `[]object` | `[...]` | | ||
| | `logsStorages[i].name` | Name of the storage instance. | `string` | `""` | | ||
| | `logsStorages[i].retentionPeriod` | Retention period for logs. | `string` | `1` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default value 1 for logsStorages[i].retentionPeriod is ambiguous. It's not clear if the unit is days, hours, or something else. Other documentation files (e.g., logs.md, setup.md) show examples with units, like "30d". To avoid confusion, please specify the unit, for example, 1d.
| | `logsStorages[i].retentionPeriod` | Retention period for logs. | `string` | `1` | | |
| | `logsStorages[i].retentionPeriod` | Retention period for logs. | `string` | `1d` | |
| | `alerta.alerts.telegram.chatID` | Telegram chat ID(s), separated by commas. | `string` | `""` | | ||
| | `alerta.alerts.telegram.disabledSeverity` | List of severities without alerts (e.g. ["informational","warning"]). | `[]string` | `[]` | | ||
| | `alerta.alerts.slack` | Configuration for Slack alerts. | `object` | `{}` | | ||
| | `alerta.alerts.slack.url` | Configuration uri for Slack alerts. | `string` | `""` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The description "Configuration uri for Slack alerts" is a bit vague. Based on other documentation files like alerting.md, this parameter is for the Slack webhook URL. Using "Webhook URL for Slack alerts" would be more precise and helpful for the user.
| | `alerta.alerts.slack.url` | Configuration uri for Slack alerts. | `string` | `""` | | |
| | `alerta.alerts.slack.url` | Webhook URL for Slack alerts. | `string` | `""` | |
Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
a755de5 to
7fc145b
Compare
Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
hack/update_apps.sh (1)
82-88: Special-case mapping for monitoring is correct.The logic properly handles the monitoring app's new structure where parameters are sourced from a dedicated include file. However, consider adding directory creation for robustness.
🔎 Suggested improvement for robustness
Add directory creation before copying to ensure the parent directory exists:
else if [[ "$app" == "monitoring" ]]; then src_file="${DEST_DIR%/}/_include/parameters.md" dest_file="${DEST_DIR%/}/monitoring/parameters.md" else src_file="${DEST_DIR%/}/_include/${app}.md" dest_file="${DEST_DIR%/}/${app}.md" fi fi # Ensure template exists (touch if missing) [[ -f "$src_file" ]] || touch "$src_file" + # Ensure destination directory exists + mkdir -p "$(dirname "$dest_file")" + # Copy template to destination (overwrite) cp "$src_file" "$dest_file"
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
content/en/docs/operations/services/_include/monitoring-overview.mdcontent/en/docs/operations/services/_include/monitoring.mdcontent/en/docs/operations/services/_include/parameters.mdcontent/en/docs/operations/services/monitoring/_index.mdcontent/en/docs/operations/services/monitoring/parameters.mdcontent/en/docs/operations/services/monitoring/setup.mdhack/update_apps.sh
💤 Files with no reviewable changes (2)
- content/en/docs/operations/services/_include/monitoring-overview.md
- content/en/docs/operations/services/_include/monitoring.md
✅ Files skipped from review due to trivial changes (1)
- content/en/docs/operations/services/_include/parameters.md
🧰 Additional context used
🪛 LanguageTool
content/en/docs/operations/services/monitoring/parameters.md
[grammar] ~71-~71: Ensure spelling is correct
Context: ... | string | replicated | ### Alerta configuration | Name ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🔇 Additional comments (4)
content/en/docs/operations/services/monitoring/setup.md (1)
5-5: LGTM! Weight adjustments align with documentation structure.The weight changes properly reorder pages to accommodate the new parameters reference page.
Also applies to: 9-9
content/en/docs/operations/services/monitoring/_index.md (1)
5-5: LGTM! Include directive simplifies content management.The shift to using an include directive aligns with the PR objectives and makes the documentation more maintainable.
content/en/docs/operations/services/monitoring/parameters.md (1)
1-117: LGTM! Comprehensive parameters reference is well-structured.The new parameters page provides clear documentation with appropriate categorization. The front matter weights correctly position this page in the navigation hierarchy.
Note: The static analysis hint about "Alerta" spelling is a false positive—Alerta is the proper name of the alerting tool.
hack/update_apps.sh (1)
82-88: No action needed. The code already handles missing include files gracefully on line 92 with[[ -f "$src_file" ]] || touch "$src_file", which creates an empty file if it doesn't exist. Additionally, the referencedparameters.mdfile exists atcontent/en/docs/operations/services/_include/parameters.md, so there is no risk of build failures from missing include files.Likely an incorrect or invalid review comment.
Signed-off-by: ohotnikov.ivan <ohotnikov.ivan@e-queo.net>
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.