docs: factual fixes from DOCS_REVIEW.md (Section A)#84
Conversation
…cycle hooks - Tracing howto: rename NewDatabaseSpan -> NewDatastoreSpan (matches the actual function in tracing/tracing.go and the code sample on the same page) - Index.md and local-dev.md: correct local-stack counts to 21 services across 18 single-service profiles plus the obs group profile (source of truth: cookiecutter docker-compose.local.yml) - production.md: new "Startup lifecycle" subsection covering CBPreStarter and CBPostStarter, symmetric to the existing shutdown coverage - architecture.md: startup and shutdown sequences now name all six lifecycle hooks (PreStart, PostStart, PreStop, FailCheck, Stop, PostStop) and link to the godoc - Index.md: new feature row advertising service lifecycle hooks - tests/content.spec.ts: new "Factual accuracy" describe block locks the corrected wording so it cannot drift again
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR updates documentation across multiple files to introduce Service Lifecycle Hooks (PreStart, PostStart, PreStop, PostStop), updates Docker Compose service counts from 19 to 21, clarifies profile terminology in local development setup, replaces deprecated tracing API references, and adds content correctness test coverage. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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. Review rate limit: 0/1 reviews remaining, refill in 22 minutes and 33 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 `@howto/Tracing.md`:
- Around line 23-25: Fix the wording in the tracing docs: change "The Package
provides function like `NewInternalSpan/NewExternalSpan/NewDatastoreSpan`" to
"the package provides functions like `NewInternalSpan`, `NewExternalSpan`, and
`NewDatastoreSpan` and update the following sentence to read that you must use
the context returned from those functions because the span is added to that
context; also replace "will not be add" with "will not be added" (refer to the
occurrences of NewInternalSpan/NewExternalSpan/NewDatastoreSpan and the
sentences about using the returned context).
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cdfbbbd0-7f0c-4629-a151-3ad1795d6805
📒 Files selected for processing (6)
Index.mdarchitecture.mdhowto/Tracing.mdhowto/local-dev.mdhowto/production.mdtests/content.spec.ts
There was a problem hiding this comment.
Pull request overview
Updates the documentation site to correct a handful of verified factual inaccuracies (function names, local-dev stack counts, and lifecycle hook coverage) and adds Playwright assertions to prevent these facts from drifting again.
Changes:
- Fix tracing howto prose to reference
NewDatastoreSpan(notNewDatabaseSpan). - Standardize local dev stack wording/counts (“21 services”, “18 single-service profiles”, plus
obsgroup profile). - Expand production + architecture docs to explicitly name startup/shutdown lifecycle hooks, and add Playwright “Factual accuracy” content tests.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/content.spec.ts | Adds Playwright content assertions to lock corrected wording/function names. |
| howto/production.md | Adds a “Startup lifecycle” section documenting CBPreStarter/CBPostStarter and ordering. |
| howto/local-dev.md | Updates local-stack counts/wording to the verified numbers. |
| howto/Tracing.md | Fixes incorrect function name in prose (NewDatastoreSpan). |
| architecture.md | Updates startup/shutdown sequences to name lifecycle hooks and link to godoc. |
| Index.md | Updates feature table (local-dev counts) and adds a lifecycle hooks feature row. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Tracing.md: fix grammar in the prose around the function names
("The Package provides function like" -> "The package provides functions
like ...", "will not be add" -> "will not be added"). Pre-existing wording
carried into this PR; flagged by CodeRabbit and Copilot.
- signals.md: align lifecycle interface table with the godoc — show
PreStart's `error` return type and call out that returning an error
aborts startup. Resolves the signature mismatch between this table and
the new "Startup lifecycle" section in production.md.
- Index.md: feature row now links to the lifecycle interfaces section
directly instead of the page-level "Shutdown Lifecycle" anchor, so the
link text reflects that the row covers both startup and shutdown hooks.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Three factual fixes from DOCS_REVIEW.md Section A. All verified against upstream
mainof each repo.A1 — Tracing howto used a non-existent function name
howto/Tracing.mdlines 23, 25 referencedNewDatabaseSpanin prose.go-coldbrew/tracing/tracing.goisNewDatastoreSpan(the same page used the correct name in the code sample at L89 and the link reference at L308).A2 — Local-stack service/profile counts were wrong on three pages
cookiecutter-coldbrew/{{cookiecutter.app_name}}/docker-compose.local.ymldefines 21 services across 18 single-service profiles plus theobsgroup profile (Prometheus, Grafana, Jaeger).Index.md:45said "19 services across 18 profiles +obsgroup" ❌howto/local-dev.md:16said "19 infrastructure services across 18 individual profiles plus one group profile" ❌quickstart.mdwas already correct.A3 — Startup lifecycle hooks were under-documented in two pages
signals.mdandreadiness.mdalready coverCBPreStarter/CBPostStarterthoroughly. The remaining gaps:howto/production.mdcovered shutdown hooks but not startup hooks → added a new Startup lifecycle subsection symmetric to the existing graceful-shutdown coverage.architecture.mdstartup/shutdown sequences listed numbered steps without naming the lifecycle hooks → both sequences now name all six hooks (PreStart,PostStart,PreStop,FailCheck,Stop,PostStop) with godoc links.Index.md"What You Get Out of the Box" → new Service Lifecycle Hooks feature row.Tests
New
tests/content.spec.ts"Factual accuracy" describe block asserts:/howto/Tracing/containsNewDatastoreSpanand notNewDatabaseSpan/howto/local-dev/contain "21 services" / "18 single-service profiles"/howto/production/contains "Startup lifecycle", "CBPreStarter", "CBPostStarter"This locks the corrected wording so the same drift can't happen again.
Test plan
bundle exec jekyll buildsucceeds with no broken-link warningsnpx playwright test tests/content.spec.ts— new "Factual accuracy" tests pass against a local Jekyll server/howto/Tracing/,/howto/local-dev/,/howto/production/,/architecture/,/Summary by CodeRabbit
Documentation
Tests