Docs overhaul: cleanup, fixes, and improvements#38
Merged
Conversation
- Rewrite landing page with production stats, feature grid, code snippet, and ASCII architecture diagram - Add quickstart tutorial (8-step, from zero to running service) - Add architecture page (request lifecycle, interceptor chain, context propagation, deployment topology) - Add FAQ page (10 common questions) - Add OpenGraph/SEO config and social preview image - Adjust nav ordering for new pages
…o feat/docs-overhaul
…config - Fix conn.Close() bug in gRPC howto (variable was pool, not conn) - Replace deprecated grpc.Dial/grpc.WithInsecure with grpc.NewClient/insecure.NewCredentials - Fix USING.md project structure and make target to match actual cookiecutter output - Add missing [open an issue] link reference in integrations.md - Fix image paths in swagger and data-builder howtos - Fix 10 spelling errors (readiness, applicable, propagation, etc.) - Normalize "Coldbrew" → "ColdBrew" across all prose text - Bump data-builder-example go.mod from Go 1.19 to 1.25 - Add nav_order to Packages.md, description fields to 4 howto pages - Enable search in _config.yml - Fix FAQ NotifyAsync → notifier.Notify to match errors howto - Add missing envconfig link reference in Packages.md
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughDocumentation and site tooling updates: health/readiness docs now describe JSON version payloads and gRPC health support; numerous branding/typo fixes and example modernizations; Go example go.mod bumped; search enabled; Playwright tests, config, package, CI workflow, and related ignore entries added. Changes
Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as CI Runner (ubuntu-latest)
participant Jekyll as Jekyll Build
participant WebServer as WebServer (serve)
participant Playwright as Playwright (Chromium)
participant Artifact as Artifact Store
GH->>Runner: workflow triggered (push/PR/dispatch)
Runner->>Jekyll: bundle exec jekyll build (JEKYLL_ENV=production)
Jekyll-->>Runner: built site (_site)
alt webServer started
Runner->>WebServer: npx serve _site --port 4000
WebServer-->>Runner: server ready
end
Runner->>Playwright: npx playwright test (uses baseURL)
Playwright->>WebServer: HTTP requests (navigation, content, links, search)
WebServer-->>Playwright: HTML / assets / responses
Playwright-->>Runner: test results
Runner->>Artifact: upload playwright-report/
Artifact-->>Runner: stored
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (1)
howto/Tracing.md (1)
14-14: Optional readability polish for the overview sentence.
Line 14 reads a bit awkwardly with slash separators and “Package” capitalization. Consider comma-separated backends and lowercase “package”.✍️ Suggested wording
-ColdBrew provides a way to add tracing to your functions using the [go-coldbrew/tracing] package. The Package implements multiple tracing backends (e.g. [New Relic] / [Opentelemetry] / [Jaeger]) which enables you to switch between them without changing your code. +ColdBrew provides a way to add tracing to your functions using the [go-coldbrew/tracing] package. The package implements multiple tracing backends (e.g., [New Relic], [Opentelemetry], and [Jaeger]), which enables you to switch between them without changing your code.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/Tracing.md` at line 14, The overview sentence is awkwardly worded with slash separators and an incorrectly capitalized "Package"; update the sentence referencing go-coldbrew/tracing to use comma-separated backend names (e.g., New Relic, OpenTelemetry, Jaeger) and lowercase "package" so it reads smoothly (mentioning go-coldbrew/tracing and the specific backend names New Relic / OpenTelemetry / Jaeger and the word "Package" to locate the text to change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@FAQ.md`:
- Around line 131-137: The snippet calls notifier.Notify(err, ctx) but never
defines ctx; update the example to declare a context value (e.g., use an
existing request/caller context or create one with
context.Background()/context.TODO()) and ensure the context package is imported
so the example compiles; reference the notifier.Notify call and show ctx being
defined before it's used.
In `@howto/Debugging.md`:
- Line 43: Fix the duplicated verb in the profiling sentence: replace "The `go
tool pprof` command can be also be used to analyze profiles..." with a
corrected phrasing such as "The `go tool pprof` command can also be used to
analyze profiles..." (or "The `go tool pprof` command is also used to analyze
profiles...") so the sentence has a single auxiliary verb and reads
grammatically; update the sentence that mentions [pprof walkthrough] and
[diagnostics doc] accordingly.
In `@howto/gRPC.md`:
- Line 25: Change the heading "Client side connection pool" to use a hyphenated
compound adjective: replace "Client side connection pool" with "Client-side
connection pool" so the document uses correct hyphenation for the compound
adjective in the heading.
- Line 21: The opening sentence on line with "Best way to get started with gRPC
in ColdBrew..." is missing the definite article; update that sentence text to
"The best way to get started with gRPC in ColdBrew is to use the ColdBrew
cookiecutter to generate a new project..." (keeping the rest of the sentence
unchanged) so it reads more naturally; edit the sentence in howto/gRPC.md
replacing the original fragment "Best way to get started..." with "The best way
to get started...".
In `@howto/index.md`:
- Line 12: Update the phrase "This section contains a collection of how to
guides for ColdBrew." by hyphenating "how-to" so it reads "This section contains
a collection of how-to guides for ColdBrew." Locate the exact string in the
document (the sentence starting with "This section contains a collection of how
to guides") and replace it with the corrected hyphenated version to use "how-to"
as a compound adjective.
In `@howto/interceptors.md`:
- Line 29: Rewrite the sentence to correct grammar and spelling: change "Its
possible to filter out response time logs message by using a [FilterFunc],
ColdBrew provides a [default filter function] implementation that filter out
common logs like healthchek, readycheck, server reflection, etc." to something
like "It's possible to filter out response-time log messages by using a
FilterFunc; ColdBrew provides a default filter function implementation that
filters out common endpoints such as healthcheck, readycheck, and server
reflection." Ensure you reference the symbols FilterFunc and default filter
function and correct spellings (healthcheck) and pluralization (messages,
filters).
In `@howto/Metrics.md`:
- Line 15: The sentence in the howto/Metrics.md README uses incorrect title
casing ("By Default"); update that sentence to use standard sentence casing by
changing "By Default" to "By default" in the paragraph that begins "ColdBrew
uses Prometheus..."; ensure the rest of the sentence and the /metrics endpoint
reference remain unchanged.
In `@howto/signals.md`:
- Line 45: Fix the typo in the documentation sentence about CBStopper: change
"If you service implements [CBStopper] interface, ColdBrew will call the `Stop`
method…" to "If your service implements [CBStopper] interface, ColdBrew will
call the `Stop` method…" so it correctly references the reader's service and
retains the CBStopper and `Stop` symbols.
In `@howto/swagger.md`:
- Line 32: Replace the empty alt text in the image markdown string
"" with a concise descriptive alt attribute (for
example: "Swagger API documentation screenshot") so the line becomes something
like "" to
improve accessibility.
- Line 22: Update the sentence "Since ColdBrew using grpc-gateway to generate
RESTful APIs, the generated Swagger / Open API specification is based on the
[grpc-gateway's Open API specification] documentation." to use correct tense by
replacing "using" with "uses" (i.e., "Since ColdBrew uses grpc-gateway to
generate RESTful APIs, ...") and ensure the clause reads smoothly; edit the line
in howto/swagger.md where that exact sentence appears.
In `@integrations.md`:
- Line 43: Replace the incorrect phrase "If you app is using" with the correct
grammar "If your app is using" wherever it appears in integrations.md (notably
the occurrences around the current diff and the lines referenced at 65, 87, and
101); update each affected sentence to use "your" so the text reads e.g. "If
your app is using [ColdBrew cookiecutter] template, initialisation is done
automatically." Ensure no other instances of the same typo remain in the file.
- Line 298: Update the link target for the "[open an issue]" anchor in
integrations.md so it points to the documentation repo's issues page
(https://github.com/go-coldbrew/docs.coldbrew.cloud/issues) instead of the core
repo; locate the anchor text "[open an issue]" in integrations.md and replace
the existing URL (https://github.com/go-coldbrew/core/issues) with the docs repo
URL to correctly route integration/feature requests to the documentation
repository.
---
Nitpick comments:
In `@howto/Tracing.md`:
- Line 14: The overview sentence is awkwardly worded with slash separators and
an incorrectly capitalized "Package"; update the sentence referencing
go-coldbrew/tracing to use comma-separated backend names (e.g., New Relic,
OpenTelemetry, Jaeger) and lowercase "package" so it reads smoothly (mentioning
go-coldbrew/tracing and the specific backend names New Relic / OpenTelemetry /
Jaeger and the word "Package" to locate the text to change).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4345b7ef-9f2d-4c54-b48c-e93174ad3d37
⛔ Files ignored due to path filters (1)
data-builder-example/go.sumis excluded by!**/*.sum
📒 Files selected for processing (21)
FAQ.mdIndex.mdPackages.mdUSING.md_config.ymlarchitecture.mddata-builder-example/go.modhowto/APIs.mdhowto/Debugging.mdhowto/Log.mdhowto/Metrics.mdhowto/Tracing.mdhowto/data-builder.mdhowto/errors.mdhowto/gRPC.mdhowto/index.mdhowto/interceptors.mdhowto/signals.mdhowto/swagger.mdintegrations.mdquickstart.md
Add 52 Playwright tests across 4 suites: - navigation: page loading, sidebar nav, TOC, CTA buttons - content: code blocks, tables, images, callouts, ASCII diagrams - search: input visibility, query results, clickable links - links: internal link validation, anchor checks, external link sampling Setup: - playwright.config.ts with webServer (serves _site/ via npx serve) - Configurable BASE_URL for testing live site or local build - CI workflow builds Jekyll then runs Playwright against static files - Gemfile updated for Ruby 4.0+ compatibility (logger, csv, ostruct, etc.)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
tests/navigation.spec.ts (1)
30-36: Variable shadowing withpagecould be clearer.The loop variable
pagefromtopLevelPagesshadows the common Playwright fixture name, requiring the rename topin the destructuring. Consider renaming the loop variable for clarity.🔧 Clearer variable naming
- for (const page of topLevelPages) { - test(`top-level page loads: ${page.path}`, async ({ page: p }) => { - const response = await p.goto(page.path); + for (const pageInfo of topLevelPages) { + test(`top-level page loads: ${pageInfo.path}`, async ({ page }) => { + const response = await page.goto(pageInfo.path); expect(response?.status()).toBe(200); - await expect(p.locator("h1").first()).toContainText(page.title); + await expect(page.locator("h1").first()).toContainText(pageInfo.title); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/navigation.spec.ts` around lines 30 - 36, Rename the loop variable that currently shadows the Playwright fixture: change the iteration variable used with topLevelPages (currently referenced as page.path and page.title) to a distinct name like topLevelPage or topPage, and update its usages inside the loop; leave the test callback fixture destructuring (page: p) and its usages (p.goto, p.locator) unchanged so there is no shadowing between the test fixture and the loop variable.Gemfile (1)
11-16: Consider alphabetical ordering for gems.Static analysis flags that gems should be sorted alphabetically within this section. This improves maintainability and makes it easier to spot duplicates.
🔧 Proposed fix for alphabetical ordering
# Required for Ruby 4.0+ (removed from default gems) -gem "logger" -gem "csv" -gem "ostruct" gem "base64" gem "bigdecimal" +gem "csv" +gem "logger" +gem "ostruct"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Gemfile` around lines 11 - 16, The listed gems (logger, csv, ostruct, base64, bigdecimal) are not alphabetically ordered; reorder these entries so they are sorted alphabetically by gem name (base64, bigdecimal, csv, logger, ostruct) within the same section while preserving any grouping or comments and keeping the exact gem declarations (gem "base64", gem "bigdecimal", gem "csv", gem "logger", gem "ostruct").tests/links.spec.ts (2)
44-46: Consider logging error details in the catch block.The empty catch block discards potentially useful diagnostic information when a request fails.
🔧 Proposed improvement
} catch { - broken.push(`${link} => failed to fetch`); + } catch (error) { + broken.push(`${link} => failed to fetch: ${error instanceof Error ? error.message : 'unknown error'}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/links.spec.ts` around lines 44 - 46, The catch block in tests/links.spec.ts currently discards the error; update the catch to accept the thrown error (e.g., catch (err)) and include the error details when recording or logging failures—append richer info to the broken array (e.g., `${link} => failed to fetch: ${err?.message || String(err)}`) and/or call console.error with the error and link to preserve diagnostics; make the change around the try/catch that pushes into broken so failures retain the exception message and stack.
73-107: External link tests may be flaky in CI.These tests make real HTTP requests to external services (pkg.go.dev, GitHub). They could fail intermittently due to:
- Network issues in CI runners
- Rate limiting from external services
- Temporary service outages
Consider adding retry logic, increasing timeout, or moving these to a separate optional test suite that runs less frequently.
// Example: Mark as slow/optional or add retry annotation test.describe("External Links (sample)", () => { test.describe.configure({ retries: 2 }); // Add retries for flaky network tests // ... existing tests });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/links.spec.ts` around lines 73 - 107, The external-link tests in the test.describe("External Links (sample)") suite (tests "pkg.go.dev links for core packages are reachable" and "GitHub repos are reachable", arrays pkgLinks and repos) are flaky in CI; update the suite to be resilient by configuring retries and a longer timeout for that describe block (use test.describe.configure to set retries e.g., 2 and use test.setTimeout or per-test timeouts) or alternatively move these tests into a separate optional/slow suite so they don’t run on every CI job; apply the change around the test.describe block that contains the two tests and arrays pkgLinks and repos..github/workflows/playwright.yml (1)
17-21: Consider pinningruby/setup-rubyto a commit SHA.Using
@v1tag is convenient but less secure than pinning to a specific commit hash. A compromised or updated tag could introduce unexpected changes.🔐 Example with pinned SHA
- name: Setup Ruby - uses: ruby/setup-ruby@v1 + uses: ruby/setup-ruby@a6e6f86333f0a2523ece813039b8b4be04560854 # v1.190.0 with: ruby-version: "3.1" bundler-cache: trueYou can find the current commit SHA for
v1at the ruby/setup-ruby releases page.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/playwright.yml around lines 17 - 21, The workflow step named "Setup Ruby" currently uses the floating tag "ruby/setup-ruby@v1"; pin this to a specific commit SHA by replacing "ruby/setup-ruby@v1" with "ruby/setup-ruby@<commit-sha>" (the uses value for the step), updating the step that contains the uses: "ruby/setup-ruby@v1" to the exact commit hash you obtain from the ruby/setup-ruby releases; keep the existing with: inputs (ruby-version and bundler-cache) unchanged and commit the updated workflow.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/playwright.yml:
- Around line 10-12: Add an explicit permissions block to the workflow to limit
the GITHUB_TOKEN scope instead of inheriting defaults: under the workflow's
top-level or inside the jobs > test definition (referencing jobs and test in the
diff), add a permissions: mapping with only the minimal rights needed for
running Playwright (for example permissions: contents: read and actions: read—or
adjust to your job's exact needs) to ensure the job uses a least-privilege
token.
In `@tests/content.spec.ts`:
- Around line 47-75: The tests "home page logo loads", "swagger image loads on
swagger howto", and "data-builder SVG loads on data-builder howto" currently use
conditional checks like if ((await img.count()) > 0) which hides missing-image
regressions; replace those conditionals with explicit assertions on the locator
counts (e.g., assert await logo.count() > 0 or use expect(await
img.count()).toBeGreaterThan(0)) so the test fails when the image is missing, or
alternatively mark the whole test with test.skip("<reason>") if the image is
intentionally optional; update the references to the locator variables (logo,
img) in each test accordingly.
---
Nitpick comments:
In @.github/workflows/playwright.yml:
- Around line 17-21: The workflow step named "Setup Ruby" currently uses the
floating tag "ruby/setup-ruby@v1"; pin this to a specific commit SHA by
replacing "ruby/setup-ruby@v1" with "ruby/setup-ruby@<commit-sha>" (the uses
value for the step), updating the step that contains the uses:
"ruby/setup-ruby@v1" to the exact commit hash you obtain from the
ruby/setup-ruby releases; keep the existing with: inputs (ruby-version and
bundler-cache) unchanged and commit the updated workflow.
In `@Gemfile`:
- Around line 11-16: The listed gems (logger, csv, ostruct, base64, bigdecimal)
are not alphabetically ordered; reorder these entries so they are sorted
alphabetically by gem name (base64, bigdecimal, csv, logger, ostruct) within the
same section while preserving any grouping or comments and keeping the exact gem
declarations (gem "base64", gem "bigdecimal", gem "csv", gem "logger", gem
"ostruct").
In `@tests/links.spec.ts`:
- Around line 44-46: The catch block in tests/links.spec.ts currently discards
the error; update the catch to accept the thrown error (e.g., catch (err)) and
include the error details when recording or logging failures—append richer info
to the broken array (e.g., `${link} => failed to fetch: ${err?.message ||
String(err)}`) and/or call console.error with the error and link to preserve
diagnostics; make the change around the try/catch that pushes into broken so
failures retain the exception message and stack.
- Around line 73-107: The external-link tests in the test.describe("External
Links (sample)") suite (tests "pkg.go.dev links for core packages are reachable"
and "GitHub repos are reachable", arrays pkgLinks and repos) are flaky in CI;
update the suite to be resilient by configuring retries and a longer timeout for
that describe block (use test.describe.configure to set retries e.g., 2 and use
test.setTimeout or per-test timeouts) or alternatively move these tests into a
separate optional/slow suite so they don’t run on every CI job; apply the change
around the test.describe block that contains the two tests and arrays pkgLinks
and repos.
In `@tests/navigation.spec.ts`:
- Around line 30-36: Rename the loop variable that currently shadows the
Playwright fixture: change the iteration variable used with topLevelPages
(currently referenced as page.path and page.title) to a distinct name like
topLevelPage or topPage, and update its usages inside the loop; leave the test
callback fixture destructuring (page: p) and its usages (p.goto, p.locator)
unchanged so there is no shadowing between the test fixture and the loop
variable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 977c59d6-139d-46a7-b998-938fb2e591cb
⛔ Files ignored due to path filters (2)
Gemfile.lockis excluded by!**/*.lockpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
.github/workflows/playwright.yml.gitignoreGemfilepackage.jsonplaywright.config.tstests/content.spec.tstests/links.spec.tstests/navigation.spec.tstests/search.spec.ts
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- package.json
- Add context import and ctx declaration to FAQ Sentry snippet - Set PAGES_REPO_NWO and JEKYLL_GITHUB_TOKEN in Playwright CI workflow so jekyll-github-metadata can resolve the repository
Update indirect dependencies to resolve CodeFactor security findings: - golang.org/x/net v0.8.0 → v0.52.0 (CVE-2023-39325) - google.golang.org/grpc v1.46.2 → v1.79.3 (GHSA-m425-mq94-257g) - golang.org/x/image v0.0.0 → v0.38.0 (CVE-2024-24792)
Grammar & spelling: - "can be also be used" → "can also be used" (Debugging.md) - "Best way" → "The best way" (gRPC.md) - "Client side" → "Client-side" (gRPC.md) - "how to guides" → "how-to guides" (index.md) - "Its possible...logs message...healthchek" → "It's possible...log messages...healthcheck" (interceptors.md) - "By Default" → "By default" (Metrics.md) - "If you service" → "If your service" (signals.md) - "ColdBrew using" → "ColdBrew uses" (swagger.md) - Add alt text to swagger image (swagger.md) - "If you app" → "If your app" (integrations.md, 4 occurrences) CI security hardening: - Add permissions: contents: read to playwright.yml - Pin ruby/setup-ruby to commit hash Tests: - Make image assertions unconditional (fail if images missing) - Handle logo rendered as CSS background by just-the-docs theme
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
howto/gRPC.md (2)
61-89:⚠️ Potential issue | 🟡 MinorMissing imports prevent example from compiling.
The example uses
context.Background()(line 80) andfmt.Println()(line 84) but doesn't import"context"or"fmt".📦 Proposed fix to add missing imports
import ( + "context" + "fmt" + "github.com/go-coldbrew/grpcpool" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/gRPC.md` around lines 61 - 89, The example in main uses context.Background() and fmt.Println() but the import block is missing "context" and "fmt"; update the imports for the main function (where grpcpool.Dial is used and client.SayHello is called) to include "context" and "fmt" alongside the existing grpc-related imports so the example compiles.
93-124:⚠️ Potential issue | 🟡 MinorMissing imports prevent example from compiling.
The example uses
context.Background()(line 115) andfmt.Println()(line 119) but doesn't import"context"or"fmt".📦 Proposed fix to add missing imports
import ( + "context" + "fmt" + "github.com/go-coldbrew/grpcpool" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/gRPC.md` around lines 93 - 124, The example's import block is missing the "context" and "fmt" packages used by main() (context.Background() and fmt.Println()), so update the import list in the snippet to include "context" and "fmt" alongside the existing imports; ensure the import section in the file that contains the main() function includes these two packages so the example compiles.
♻️ Duplicate comments (1)
integrations.md (1)
298-298:⚠️ Potential issue | 🟡 MinorRoute “[open an issue]” to the docs repo issues page.
Line 298 still points to
go-coldbrew/core/issues, but this CTA is in docs and appears to be for integrations requests from docs users. Please switch it to the docs repository issues URL.🔗 Suggested edit
-[open an issue]: https://github.com/go-coldbrew/core/issues +[open an issue]: https://github.com/go-coldbrew/docs.coldbrew.cloud/issues🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations.md` at line 298, Update the "[open an issue]" link target in integrations.md so it points to the docs repository issues page instead of go-coldbrew/core; locate the markdown link with link text "[open an issue]" and replace its URL (currently https://github.com/go-coldbrew/core/issues) with the docs repo issues URL (e.g., https://github.com/go-coldbrew/docs/issues) so the CTA routes docs users to the correct issues tracker.
🧹 Nitpick comments (1)
howto/gRPC.md (1)
38-38: Consider adding a security note for insecure credentials.All three examples use
insecure.NewCredentials(). While this is appropriate for documentation examples, consider adding a note that production deployments should use proper TLS credentials.💡 Suggested addition
Add a note block after line 31 or in the first example:
{: .warning} The examples use `insecure.NewCredentials()` for simplicity. In production, use proper TLS credentials with `credentials.NewClientTLSFromCert()` or `credentials.NewClientTLSFromFile()`.Also applies to: 66-66, 98-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/gRPC.md` at line 38, The examples use insecure.NewCredentials() without warning; add a short security note after the first example (or after line 31) explaining that insecure.NewCredentials() is for examples only and production should use proper TLS credentials (e.g., credentials.NewClientTLSFromCert or credentials.NewClientTLSFromFile); apply the same note near the other occurrences of insecure.NewCredentials() in the file (the examples around lines 66 and 98) so readers know to replace insecure creds with real TLS in production.
🤖 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/signals.md`:
- Line 5: Update the YAML description metadata to use the singular phrasing
“POSIX signal handling” instead of “signals handling”; specifically change the
description value in howto/signals.md to read "How POSIX signal handling works
in ColdBrew" so the metadata is grammatically correct.
- Line 56: Replace the verb phrase "When you shutdown your application," in
howto/signals.md with the correct verb form "When you shut down your
application," and ensure any other occurrences in that paragraph use "shut down"
(verb) rather than "shutdown" (noun/adjective).
In `@integrations.md`:
- Line 10: The sentence "ColdBrew is designed to be very thin wrappers over
other services and tools." has a subject/verb number mismatch; change it to a
grammatically correct form such as "ColdBrew is designed to be a very thin
wrapper over other services and tools." (or, if you prefer plural, revise the
subject to something like "ColdBrew components are designed to be thin
wrappers..."); update the sentence in integrations.md where the current wording
appears.
---
Outside diff comments:
In `@howto/gRPC.md`:
- Around line 61-89: The example in main uses context.Background() and
fmt.Println() but the import block is missing "context" and "fmt"; update the
imports for the main function (where grpcpool.Dial is used and client.SayHello
is called) to include "context" and "fmt" alongside the existing grpc-related
imports so the example compiles.
- Around line 93-124: The example's import block is missing the "context" and
"fmt" packages used by main() (context.Background() and fmt.Println()), so
update the import list in the snippet to include "context" and "fmt" alongside
the existing imports; ensure the import section in the file that contains the
main() function includes these two packages so the example compiles.
---
Duplicate comments:
In `@integrations.md`:
- Line 298: Update the "[open an issue]" link target in integrations.md so it
points to the docs repository issues page instead of go-coldbrew/core; locate
the markdown link with link text "[open an issue]" and replace its URL
(currently https://github.com/go-coldbrew/core/issues) with the docs repo issues
URL (e.g., https://github.com/go-coldbrew/docs/issues) so the CTA routes docs
users to the correct issues tracker.
---
Nitpick comments:
In `@howto/gRPC.md`:
- Line 38: The examples use insecure.NewCredentials() without warning; add a
short security note after the first example (or after line 31) explaining that
insecure.NewCredentials() is for examples only and production should use proper
TLS credentials (e.g., credentials.NewClientTLSFromCert or
credentials.NewClientTLSFromFile); apply the same note near the other
occurrences of insecure.NewCredentials() in the file (the examples around lines
66 and 98) so readers know to replace insecure creds with real TLS in
production.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 512d1ecc-6594-4f40-bf9c-172daacb643a
⛔ Files ignored due to path filters (1)
data-builder-example/go.sumis excluded by!**/*.sum
📒 Files selected for processing (12)
.github/workflows/playwright.ymlFAQ.mddata-builder-example/go.modhowto/Debugging.mdhowto/Metrics.mdhowto/gRPC.mdhowto/index.mdhowto/interceptors.mdhowto/signals.mdhowto/swagger.mdintegrations.mdtests/content.spec.ts
✅ Files skipped from review due to trivial changes (4)
- howto/Debugging.md
- howto/swagger.md
- howto/index.md
- howto/Metrics.md
🚧 Files skipped from review as they are similar to previous changes (5)
- howto/interceptors.md
- FAQ.md
- .github/workflows/playwright.yml
- data-builder-example/go.mod
- tests/content.spec.ts
Member
Author
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes and stabilizes the ColdBrew docs site by cleaning up/standardizing documentation content, enabling just-the-docs search, and adding Playwright-based smoke tests (plus CI) to catch broken pages/links/assets early.
Changes:
- Enabled site search and improved doc page front-matter (descriptions/nav ordering) across core pages.
- Updated/normalized multiple how-to and reference docs (fixes to links, images, wording, examples, and branding).
- Added Playwright smoke tests + npm tooling and a GitHub Actions workflow to build the Jekyll site and validate navigation/search/links/content.
Reviewed changes
Copilot reviewed 29 out of 33 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/search.spec.ts | Adds Playwright coverage for search input visibility and basic search result rendering. |
| tests/navigation.spec.ts | Adds page-load and sidebar/TOC/CTA navigation smoke tests. |
| tests/links.spec.ts | Adds internal link crawl, anchor existence checks, and sampled external link reachability. |
| tests/content.spec.ts | Adds smoke checks for key content elements (code blocks, tables, images, callouts, diagrams). |
| playwright.config.ts | Configures Playwright (baseURL, CI retries, local webServer serving _site). |
| package.json | Introduces npm scripts for building Jekyll and running Playwright tests. |
| package-lock.json | Locks Playwright/serve dependencies for repeatable CI/local installs. |
| .github/workflows/playwright.yml | Adds CI workflow to build the Jekyll site and run Playwright tests on PRs/pushes. |
| .gitignore | Ignores Node/Playwright artifacts (node_modules, reports, test-results) alongside _site. |
| _config.yml | Enables just-the-docs search (search_enabled: true). |
| Index.md | Updates endpoint descriptions (health/ready probes now described as returning version JSON). |
| quickstart.md | Updates quickstart examples/output (health/ready JSON output; Go version menu ordering). |
| architecture.md | Updates health/readiness semantics and documents standard gRPC health protocol support. |
| integrations.md | Fixes/normalizes branding and adds missing [open an issue] link reference. |
| Packages.md | Adds nav ordering, branding normalization, and missing [envconfig] reference link. |
| USING.md | Updates documented cookiecutter-generated project structure and make generate command. |
| FAQ.md | Updates error notification example to use errors/notifier API. |
| howto/index.md | Wording/branding tweak for how-to section intro. |
| howto/APIs.md | Branding normalization + typo fixes + link label updates. |
| howto/gRPC.md | Branding normalization + updates examples to newer gRPC client APIs and pool closing. |
| howto/interceptors.md | Adds page description, branding fixes, and updates gRPC client example to modern APIs. |
| howto/errors.md | Fixes heading typo (“providers”). |
| howto/Tracing.md | Branding normalization within overview text. |
| howto/Metrics.md | Adds page description and improves wording/branding consistency. |
| howto/Log.md | Adds page description. |
| howto/Debugging.md | Adds page description and fixes a grammar issue in pprof section. |
| howto/signals.md | Adds page description, fixes typos (“readiness/propagation/useful”), and branding consistency. |
| howto/swagger.md | Adds branding consistency, fixes applicability typo, and corrects image path/alt text. |
| howto/data-builder.md | Fixes typo (“structs”) and corrects image path. |
| data-builder-example/go.mod | Bumps module Go version and updates indirect dependency versions. |
| data-builder-example/go.sum | Updates dependency checksums to match the bumped module versions. |
| Gemfile | Adds Ruby stdlib gems explicitly (for newer Ruby distributions where they’re not default). |
| Gemfile.lock | Updates lockfile to reflect added Ruby gems/platform entries. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- "POSIX signals handling" → "POSIX signal handling" (signals.md) - "When you shutdown" → "When you shut down" (signals.md) - "is designed to be very thin wrappers" → "provides thin wrappers" (integrations.md) - "The Package" → "The package" (Tracing.md) - Rename link test "return 200" → "resolve without errors" (links.spec.ts)
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
integrations.md (1)
298-298:⚠️ Potential issue | 🟡 MinorRoute
[open an issue]to the docs repository issues page.This still points to
go-coldbrew/core/issues, but this page’s call-to-action appears docs-scoped and was previously flagged. Consider updating tohttps://github.com/go-coldbrew/docs.coldbrew.cloud/issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@integrations.md` at line 298, The "[open an issue]" link in integrations.md currently routes to the core repo (https://github.com/go-coldbrew/core/issues); change the URL to the docs repository issues page (https://github.com/go-coldbrew/docs.coldbrew.cloud/issues) and search for any other occurrences of the same link text or the core/issues URL to update them as well so all "open an issue" CTAs point to the docs repo.
🧹 Nitpick comments (1)
howto/Tracing.md (1)
17-17: Minor grammar fix: missing apostrophe.Since this PR focuses on documentation cleanup and typo fixes, consider correcting "Its possible" to "It's possible" (contraction requires an apostrophe).
📝 Proposed fix
-Its possible for you to have multiple backends enabled at the same time, for example you can have both [New Relic] and [Opentelemetry] enabled at the same time in the same span and they will both receive the same trace. +It's possible for you to have multiple backends enabled at the same time, for example you can have both [New Relic] and [Opentelemetry] enabled at the same time in the same span and they will both receive the same trace.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@howto/Tracing.md` at line 17, Fix the missing apostrophe in the sentence "Its possible for you to have multiple backends enabled at the same time, for example you can have both [New Relic] and [Opentelemetry] enabled at the same time in the same span and they will both receive the same trace." by changing "Its possible" to "It's possible" so the contraction is correct; update the exact sentence in howto/Tracing.md where that phrase appears.
🤖 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/signals.md`:
- Line 19: The sentence "ColdBrew applications are built on top of
[go-coldbrew/core] package, have the ability to handle [POSIX signals]." has a
comma splice; fix it by either removing the comma to make it restrictive
("ColdBrew applications are built on top of [go-coldbrew/core] package and have
the ability to handle [POSIX signals].") or by adding an explicit relative
clause to make it non‑restrictive ("ColdBrew applications, which are built on
top of the [go-coldbrew/core] package, have the ability to handle [POSIX
signals]."); update the line in howto/signals.md accordingly.
---
Duplicate comments:
In `@integrations.md`:
- Line 298: The "[open an issue]" link in integrations.md currently routes to
the core repo (https://github.com/go-coldbrew/core/issues); change the URL to
the docs repository issues page
(https://github.com/go-coldbrew/docs.coldbrew.cloud/issues) and search for any
other occurrences of the same link text or the core/issues URL to update them as
well so all "open an issue" CTAs point to the docs repo.
---
Nitpick comments:
In `@howto/Tracing.md`:
- Line 17: Fix the missing apostrophe in the sentence "Its possible for you to
have multiple backends enabled at the same time, for example you can have both
[New Relic] and [Opentelemetry] enabled at the same time in the same span and
they will both receive the same trace." by changing "Its possible" to "It's
possible" so the contraction is correct; update the exact sentence in
howto/Tracing.md where that phrase appears.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 294e4100-eac4-45b6-92cd-f8140e627105
📒 Files selected for processing (4)
howto/Tracing.mdhowto/signals.mdintegrations.mdtests/links.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/links.spec.ts
| ## Overview | ||
|
|
||
| Coldbrew applications are built on top of [go-coldbrew/core] package, have the ability to handle [POSIX signals]. | ||
| ColdBrew applications are built on top of [go-coldbrew/core] package, have the ability to handle [POSIX signals]. |
There was a problem hiding this comment.
Fix sentence structure for clarity.
The comma after "package" creates an awkward sentence structure. Either remove the comma to make it a restrictive clause, or add "which are" to make it a proper non-restrictive clause.
📝 Suggested grammar fix
Option 1 (preferred, more concise):
-ColdBrew applications are built on top of [go-coldbrew/core] package, have the ability to handle [POSIX signals].
+ColdBrew applications built on top of [go-coldbrew/core] package have the ability to handle [POSIX signals].Option 2 (explicit relative clause):
-ColdBrew applications are built on top of [go-coldbrew/core] package, have the ability to handle [POSIX signals].
+ColdBrew applications, which are built on top of [go-coldbrew/core] package, have the ability to handle [POSIX signals].📝 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.
Suggested change
| ColdBrew applications are built on top of [go-coldbrew/core] package, have the ability to handle [POSIX signals]. | |
| ColdBrew applications built on top of [go-coldbrew/core] package have the ability to handle [POSIX signals]. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@howto/signals.md` at line 19, The sentence "ColdBrew applications are built
on top of [go-coldbrew/core] package, have the ability to handle [POSIX
signals]." has a comma splice; fix it by either removing the comma to make it
restrictive ("ColdBrew applications are built on top of [go-coldbrew/core]
package and have the ability to handle [POSIX signals].") or by adding an
explicit relative clause to make it non‑restrictive ("ColdBrew applications,
which are built on top of the [go-coldbrew/core] package, have the ability to
handle [POSIX signals]."); update the line in howto/signals.md accordingly.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 33 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Mar 24, 2026
5 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Comprehensive documentation cleanup across 19 files:
conn.Close()bug in gRPC howto, replaced deprecatedgrpc.Dial/grpc.WithInsecurewith modern APIs, fixed USING.md project structure to match actual cookiecutter output, fixedmake gen→make generate[open an issue]link in integrations.md, fixed image paths in swagger/data-builder howtos, added missing[envconfig]link in Packages.mdNotifyAsync→notifier.Notifyto match errors howtodescriptionfields to 4 howto pages,nav_orderto Packages.md, enabled site searchTest plan
./run-local.sh) and verify no broken pagessearch_enabled: true/howto/gRPC/,/howto/signals/,/using//howto/swagger/and/howto/data-builder/[open an issue]link works on/integrations/cd data-builder-example && go build ./...to verify go.mod is validSummary by CodeRabbit