Conversation
asoorm
requested review from
JivusAyrus,
StarpTech,
alepane21 and
thisisnithin
and removed request for
a team
asoorm
changed the title
asoorm
force-pushed
the
ahmet/eng-8587-documentation-for-connect-client-v2
branch
2 times, most recently
from
eb327e5 to
d4818df
Compare
asoorm
force-pushed
the
ahmet/eng-8587-documentation-for-connect-client-v2
branch
5 times, most recently
from
4eaca36 to
74f6ac9
Compare
StarpTech
left a comment
StarpTech
left a comment
There was a problem hiding this comment.
Further feedback
- We should follow one design schema in diagrams. I'd adjust the diagram /connect-rpc/overview to the style in http://localhost:3000/connect/overview#deployment-models
I really like the new structure 👍
asoorm
force-pushed
the
ahmet/eng-8587-documentation-for-connect-client-v2
branch
from
3577709 to
fca0111
Compare
WalkthroughAdds a new "Cosmo ConnectRPC" documentation section (seven pages) and updates multiple internal doc links and navigation entries to reflect the reorganized docs structure. No source code or exported/public API declarations were changed. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 5
🤖 Fix all issues with AI agents
In `@docs/connect-rpc/consume-via-rest-openapi.mdx`:
- Around line 62-78: The "HTTP GET Requests (Caching)" heading is incorrectly an
h3 and appears nested under "HTTP POST requests"; change the heading "### HTTP
GET Requests (Caching)" to h2 ("## HTTP GET Requests (Caching)") so GET is a
peer of POST, and also disambiguate the duplicate "### Examples" headings by
renaming the second one (e.g., "### Examples (GET)" or "### Examples — GET
Requests") to avoid duplicate anchors; update any internal links or refs that
rely on the old anchor text if present.
In `@docs/connect-rpc/consume-via-sdks.mdx`:
- Around line 39-58: The example uses the deprecated createPromiseClient from
`@connectrpc/connect`; update the import and client creation to use createClient
instead: replace the import symbol createPromiseClient with createClient and
change the call site where client is instantiated (currently client =
createPromiseClient(HrService, transport)) to client = createClient(HrService,
transport); keep the existing createConnectTransport usage and HrService
reference unchanged so types and transport configuration remain the same.
In `@docs/connect-rpc/produce-configure-router.mdx`:
- Around line 55-67: The docker run example contains inline Bash comments
between backslash-continued lines which breaks the command; update the docs by
removing those inline `#` comments from the multi-line `docker run \` block (or
move explanatory text above the block) so each continued line ends with a
backslash and only valid flags like `-p 3002:3002`, `-p 5026:5026`, `-v
"$(pwd)/config.yaml:/config/config.yaml:ro"`, and `-v
"$(pwd)/services:/services:ro"` remain in the code block, ensuring the final
image reference `ghcr.io/wundergraph/cosmo/router:latest` executes correctly
when copy-pasted.
In `@docs/connect-rpc/produce-define-contracts.mdx`:
- Line 91: Update the link target in the sentence "For the complete list of
available command flags and options, see the [CLI reference
documentation](/cli/essentials)." to point to the specific CLI command reference
for the grpc-service generate command instead of the general CLI overview;
replace the href /cli/essentials with the grpc-service generate command page URL
(the page for the "grpc-service generate" CLI command) while keeping the link
text or update it to explicitly mention "grpc-service generate" for clarity.
In `@docs/connect-rpc/produce-generate-distribute-sdks.mdx`:
- Around line 45-49: Replace the incorrect Go module path used to install the
protoc-gen-connect-openapi plugin: update the install command that references
connectrpc.com/connect/cmd/protoc-gen-connect-openapi@latest to use the
community module github.com/sudorandom/protoc-gen-connect-openapi@latest so the
protoc-gen-connect-openapi tool installs correctly.
🧹 Nitpick comments (3)
docs/connect-rpc/consume-via-grpc.mdx (1)
70-74: Add a short “See also” to cross-link the other ConnectRPC consumption paths.This page already links to SDKs, but adding explicit links to the overview and REST/OpenAPI pages would make it easier to compare consumption options.
As per coding guidelines: Link between related documentation pages.Suggested addition
<Note> For the best developer experience in languages like TypeScript, Go, and Kotlin, we recommend using the pre-generated [SDKs](/connect-rpc/consume-via-sdks) instead of raw gRPC clients. </Note> + +See also: [Overview](/connect-rpc/overview) · [Consume via REST/OpenAPI](/connect-rpc/consume-via-rest-openapi) · [Consume via SDKs](/connect-rpc/consume-via-sdks)docs/connect-rpc/produce-define-contracts.mdx (1)
7-24: Custom diff CSS may conflict with or duplicate Mintlify's built-in styling.Embedding raw
<style>blocks in MDX is fragile — it applies globally and could clash with theme updates. If Mintlify's code blocks already supportdiffsyntax highlighting, this may be unnecessary. If it is needed, consider whether a shared CSS file or Mintlify's custom CSS config would be more maintainable.docs/connect-rpc/produce-generate-distribute-sdks.mdx (1)
58-66: Update buf configuration to v2 format.Both
buf.yamlandbuf.gen.yamluseversion: v1. Buf introduced a v2 configuration format with a cleaner, more modular structure (e.g.,modulesinbuf.yaml,inputsinbuf.gen.yaml). Since this is new documentation targeting users, updating to v2 would provide a more modern example and avoid pointing them toward a legacy configuration format.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/connect-rpc/consume-via-rest-openapi.mdx`:
- Around line 62-76: The heading "#### Constructing the GET request" under "##
HTTP GET Requests (Caching)" skips an h3 level; update that heading to "###
Constructing the GET request" so the document respects proper hierarchy and
TOC/accessibility; locate the heading line containing the exact text
"Constructing the GET request" and change its Markdown level from #### to ###
(ensure any related subheadings remain consistent).
In `@docs/connect-rpc/produce-define-contracts.mdx`:
- Line 177: The MDX heading "## 4. Versioning & Stability" uses an HTML
entity and may render as "&"; update the heading to use a literal ampersand
by replacing "&" with "&" (i.e., "## 4. Versioning & Stability") so the
MDX/JSX processor renders the correct character; locate the heading text in
produce-define-contracts.mdx and make the substitution.
🧹 Nitpick comments (3)
docs/connect-rpc/consume-via-sdks.mdx (1)
78-110: Go example lacks nil-safety, unlike the TypeScript counterpart.Line 109 dereferences
res.Msg.Employee.Details.Forenamewithout nil checks, which will panic if any intermediate field is nil. The TypeScript example (line 54) correctly uses optional chaining (response.employee?.details?.forename). Consider adding a nil guard or a brief comment so consumers don't copy-paste a panic-prone pattern.Suggested change
- // Response data is strongly typed - fmt.Printf("Fetched employee: %s\n", res.Msg.Employee.Details.Forename) + // Response data is strongly typed; check for nil if fields are optional + if emp := res.Msg.GetEmployee(); emp != nil { + fmt.Printf("Fetched employee: %s\n", emp.GetDetails().GetForename()) + }docs/connect-rpc/produce-define-contracts.mdx (2)
250-258: Inconsistent inline code formatting in Best Practices.
service.proto.lock.json,.graphql,.proto, andwgcare referenced without backtick formatting here, unlike the rest of the document where they are consistently formatted as inline code.Proposed fix
-1. Commit the lock file: Always commit service.proto.lock.json to your version control system along with your .graphql and .proto files. +1. Commit the lock file: Always commit `service.proto.lock.json` to your version control system along with your `.graphql` and `.proto` files. -2. Do not edit manually: Never manually modify or delete the lock file. Let the wgc CLI manage it. +2. Do not edit manually: Never manually modify or delete the lock file. Let the `wgc` CLI manage it.
7-24: Inline<style>block for diff highlighting is fragile.This works but ties the rendering to specific CSS class names from the syntax highlighter (Prism's
.token.inserted/.token.deleted). If the docs platform changes its highlighter or class names, this will silently break. Consider checking whether the documentation framework (Mintlify?) has native diff highlighting support.
asoorm
force-pushed
the
ahmet/eng-8587-documentation-for-connect-client-v2
branch
from
e4c3726 to
2b256b2
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/cli/router/plugin/build.mdx (1)
39-39:⚠️ Potential issue | 🟡 MinorPre-existing typo: "TyppeScript" → "TypeScript".
While not part of this diff, since this file is being touched it's a good opportunity to fix.
✏️ Suggested fix
-1. **Detect Plugin Language For Building**: The language of the plugin is detected based on the presence of a go.mod file for Go, if it does not exist we check for a `package.json` file for TyppeScript, if none of these condition match the command fails +1. **Detect Plugin Language For Building**: The language of the plugin is detected based on the presence of a go.mod file for Go, if it does not exist we check for a `package.json` file for TypeScript, if none of these conditions match the command fails
🤖 Fix all issues with AI agents
In `@docs/connect-rpc/produce-configure-router.mdx`:
- Around line 28-40: The YAML uses a relative path ("path: \"./services\"")
under storage_providers.file_system for the fs-services provider while the
Docker mount targets an absolute path (/services), which can cause a runtime
mismatch; update the storage_providers.file_system entry for id "fs-services" to
use the absolute path "/services" (or explicitly note that the container WORKDIR
must be "/" in the docs) and ensure connect_rpc.storage.provider_id remains
"fs-services" so the router points to the corrected path.
- Line 15: Replace the plain text "Define Contracts step" with an MDX link to
the prerequisite documentation page: find the string "Define Contracts step" in
produce-configure-router.mdx and change it to a linked form (e.g. [Define
Contracts](<relative-path-to-define-contracts-page>)) using the repo's standard
MDX/Markdown link style so the text points to the "Define Contracts" doc; ensure
the link target uses the correct relative path or route for the existing "Define
Contracts" page.
In `@docs/connect-rpc/produce-define-contracts.mdx`:
- Around line 215-221: The JSON diff under "fields" is missing a comma after the
"email": 2 entry which makes the example invalid; update the example in
produce-define-contracts.mdx so that the "email": 2 line ends with a comma
before adding the "phone": 3 entry (i.e., ensure the "fields" object entries
such as "name", "email", and "phone" are comma-separated).
In `@docs/connect-rpc/produce-generate-distribute-sdks.mdx`:
- Line 134: Replace the HTML entity in the MDX heading so it renders correctly:
locate the heading "## 4. Distribution Strategy & CI/CD Integration" and
change it to use a literal ampersand, i.e., "## 4. Distribution Strategy & CI/CD
Integration", ensuring no HTML entities remain in that heading line.
- Around line 58-66: Update the buf.yaml example from the v1 schema to the v2
schema by replacing the top-level "version: v1" with "version: v2" and switching
to the v2 keys (e.g., add a "modules:" list that enumerates module paths instead
of relying on implicit roots), or alternatively add a short note next to the
shown buf.yaml that the snippet is v1 and link to the Buf migration command "buf
config migrate" and the migration guide; reference the example file name
"buf.yaml" and ensure the doc mentions the v2 "modules" key so readers using Buf
CLI v1.32.0+ know to migrate.
🧹 Nitpick comments (5)
docs/connect-rpc/consume-via-sdks.mdx (1)
7-15: Add cross-links to related documentation pages.Lines 11 and 15 reference the SDK generation/distribution workflow and package names provided by the platform team, but neither links to the sibling page. Consider adding a link to help readers navigate the producer workflow.
Proposed fix
-Once your platform team has generated and distributed the client SDKs, consuming the API is straightforward. +Once your platform team has [generated and distributed the client SDKs](/connect-rpc/produce-generate-distribute-sdks), consuming the API is straightforward.As per coding guidelines: "Link between related documentation pages."
docs/connect-rpc/produce-generate-distribute-sdks.mdx (1)
118-118: Add a cross-link to the "Define Contracts" page.The text references "the setup in Define Contracts" but doesn't link to the corresponding page. Readers arriving at this page directly won't know where to find that content.
Proposed fix
-Buf will discover the `.proto` files in your `services/` directory (based on the setup in Define Contracts) and run the configured plugins. +Buf will discover the `.proto` files in your `services/` directory (based on the setup in [Define Contracts](/connect-rpc/produce-define-contracts)) and run the configured plugins.As per coding guidelines: "Link between related documentation pages."
docs/connect-rpc/produce-define-contracts.mdx (3)
7-24: Inline<style>block for diff highlighting — consider alternatives.Embedding raw CSS for Prism token styling is brittle and may not survive theme or doc-platform changes. If your docs framework (e.g., Mintlify, Docusaurus) supports diff-language code blocks natively, you could remove this block entirely. If not, this works but is worth noting as tech debt.
250-258: Wrap file names and CLI tool names in backticks for consistency.Lines 252 and 254 mention
service.proto.lock.jsonandwgcas plain text, while the rest of the document consistently uses inline code formatting for these identifiers.Proposed fix
-1. Commit the lock file: Always commit service.proto.lock.json to your version control system along with your .graphql and .proto files. +1. Commit the lock file: Always commit `service.proto.lock.json` to your version control system along with your `.graphql` and `.proto` files. -2. Do not edit manually: Never manually modify or delete the lock file. Let the wgc CLI manage it. +2. Do not edit manually: Never manually modify or delete the lock file. Let the `wgc` CLI manage it.
26-30: Add a forward link to the next workflow step.This page covers the first step (defining contracts). At the end of the page or in the intro, consider linking to the next step — Generate & Distribute SDKs — to guide readers through the workflow sequence.
As per coding guidelines: "Link between related documentation pages." Based on learnings: "Link between related documentation pages."
- Add ConnectRPC overview with value proposition, architecture diagrams, and multi-protocol examples - Add producer guides: define contracts, configure router, generate & distribute SDKs - Add consumer guides: REST/OpenAPI, gRPC, and typed SDK consumption - Fix deprecated createPromiseClient → createClient in TypeScript examples - Fix broken docker run command by removing inline comments - Fix heading hierarchy, duplicate anchors, and HTML entities - Align mermaid diagrams with existing docs design system - Correct CLI reference link and protoc-gen-connect-openapi install path
asoorm
force-pushed
the
ahmet/eng-8587-documentation-for-connect-client-v2
branch
from
2b256b2 to
147fa6b
Compare
2 participants
This PR adds a complete documentation set for Cosmo Connect API consumption, covering both the producer workflow and consumer experience.
What's included
Producer workflow
Consumption options
Structure & intent
Summary by CodeRabbit