Repository navigation
docs(agents): define where a new document belongs - #3777
myasnikovdaniil wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe documentation updates add guidance for selecting documentation destinations and revise a CloudNativePG upgrade note to state that the ChangesDocumentation placement guidance
Backup upgrade note
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Merge Risk: 🔵 Low · up to The documentation may leave authors unclear about the automated website PR, but the workflow itself is correct and the impact is limited to guidance clarity. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/agents/overview.md`:
- Line 139: Update the documentation guidance in the README publishing sentence
to clarify that the website pull request is automated, replacing “without a
second PR” with wording such as “without a second manual PR by the author.”
Preserve the existing explanation of the website update targets and package
README workflow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 74d71a77-500b-4e7c-8d89-8bca4d2e0740
📒 Files selected for processing (2)
docs/agents/overview.mddocs/operations/backup-classes.md
|
|
||
| | Destination | What belongs there | | ||
| | --- | --- | | ||
| | `packages/<group>/<name>/README.md` | Anything scoped to a single package. Prose goes **above** `## Parameters`; that section is `cozyvalues-gen` output and is never hand-edited. The website's `make update-apps` / `update-vms` / `update-networking` / `update-k8s` / `update-services` targets append the whole README onto a published page, so this route reaches users without a second PR. Prefer it whenever the content fits one package. | |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Clarify the second pull request requirement.
docs/release.md states that the website-docs job opens a separate cozystack/website documentation pull request. Replace “without a second PR” with “without a second manual PR by the author,” or state that the website PR is automated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/agents/overview.md` at line 139, Update the documentation guidance in
the README publishing sentence to clarify that the website pull request is
automated, replacing “without a second PR” with wording such as “without a
second manual PR by the author.” Preserve the existing explanation of the
website update targets and package README workflow.
Aleksei Sviridkin (lexfrei)
left a comment
There was a problem hiding this comment.
myasnikovdaniil NOT LGTM. The rule is still needed on current main, where overview.md has the old four-line list and docs/ still carries the GPU, OIDC and VLAN guides, but two rows of the new table send people to the wrong place.
The version rule for new website pages contradicts the website's own guide. The table says a new page goes into both next/ and the current released version. The website's CONTRIBUTING.md says a doc for a feature that ships in a future release goes into next/ only, and the current version directory is for fixes to what is already released. A doc written in the same PR as the feature is almost always the first case, so following this row would publish docs for an unreleased feature on the production site. I'd point at the website's CONTRIBUTING.md instead of restating its rules here, since a copy will drift.
The package README route only works for some packages. The website's update_apps.sh fetches packages/apps/<name>/README.md or packages/extra/<name>/README.md, and only for names in the APPS, VMS, NETWORKING, K8S and SERVICES lists in its Makefile. Nothing under packages/system/ is ever imported, and neither are apps like bucket, opensearch or valkey, or extras like gateway and external-dns. The sync is also a manual make update-* run in the website repo, not something that happens on merge. As written, "prefer it whenever the content fits one package" would send user docs for a system package into a README nobody reads. The row needs to name that limit.
The commit carries Assisted-By: Claude <[email protected]>. docs/agents/contributing.md takes only Assisted-by: LLM there.
The rest checks out against main. seaweedfs-431-rename-recovery.md is referenced by both seaweedfs _naming.tpl files and migration 53, both DESIGN.md paths exist, and the backup-classes.md edit is correct: postgres-operator/values.yaml no longer pins image.tag. The branch merges cleanly into current main and pre-commit is green.
The `docs/` tree had accumulated component guides that belong on the website: standalone OIDC, GPU/vGPU and VM-networking documents written as part of a feature PR and never routed anywhere. Nothing said where documentation should go, and in particular nothing mentioned that the website appends a whole package README onto a published page, so a contributor with package-scoped prose had no reason to use that route. Replace the four-line "Documentation is organized as follows" list with a destination table and the test that separates `docs/` from the website: whether a non-documentation file has to link to the document by path. Also state that a merged document records current behaviour and its limitations, not the development process around it — the "What is NOT in Phase N" and "this PR" residue that motivated this. Drop one such residue while here: backup-classes.md described a pin as one "this PR removes". Assisted-by: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
f98f455 to
1640405
Compare
What this PR does
docs/had collected component guides that belong on the website, standalone oidc, gpu and vm networking documents written inside the feature PRs that added the code and never routed anywhere a user looks. Nothing said where documentation should go, and in particular nothing mentioned that the website appends a whole package README onto a published page, so a contributor with package scoped prose had no reason to use that route.This replaces the four line "Documentation is organized as follows" list in
docs/agents/overview.mdwith a destination table and the test that separatesdocs/from the website: whether a non-documentation file has to link to the document by path. It also says that a merged document records current behaviour and its limitations, not the development process around it, which is the "What is NOT in Phase N" and "this PR" residue that motivated the whole thing.One such residue dropped while here,
docs/operations/backup-classes.mddescribed a pin as one "this PR removes".Moving the documents themselves is a separate PR.
Downstream repositories
Walked the trigger map against the diff: this touches
docs/agents/overview.mdanddocs/operations/backup-classes.mdonly. No package added, renamed or removed, novalues.yamlorvalues.schema.jsonfield, nopackages/core/platform/values.yamlkey, no bundle or variant, no release asset name, noApplicationDefinitionsemantics, no Talos pin, and no change tocozyvalues-gen,cozypkgor a package Makefile.Release note
Summary by CodeRabbit