-
Notifications
You must be signed in to change notification settings - Fork 272
docs: add feature and troubleshooting documentation #463
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
793be74 to
3a76d85
Compare
|
There are a lot of lines in this CL, but many of them are duplicate. Things to review:
Thanks! P.S. I didn't get to everything, but the CL was already getting quite large, so we should review it. |
Add a framework for feature documentation, and start populating it with our SDK documentation. This framework is as follows: - internal/docs/**.src.md is the markdown source for the docs/ directory. - The x/example/internal/cmd/weave tool is used to compile these docs to the top-level docs, supporting both linked code samples and generated tables of contents. - The readme-check workflow is updated to check these docs as well. - The structure of these docs follows the MCP spec. - Wherever possible, example code is linked from actual Go documentation examples, so that it is testable. Some minor modifications to the weave tool were made to support this framework. Additionally, partially fill out this documentation with content on base protocol and client features, as well as troubleshooting help. Along the way, a bug was encountered that our LoggingTransport was not concurrency safe. This is fixed with a mutex. Fixes modelcontextprotocol#466 Fixes modelcontextprotocol#409 Updates modelcontextprotocol#442
internal/docs/protocol.src.md
Outdated
| transport := &mcp.StreamableClientTransport{ | ||
| Endpoint: "http://localhost:8080/mcp", | ||
| } | ||
| client, err := mcp.Connect(context.Background(), transport, &mcp.ClientOptions{...}) |
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.
Use ctx here.
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.
Done
internal/docs/protocol.src.md
Outdated
| ```go | ||
| ctx, cancel := context.WithCancel(context.Background()) | ||
| go cs.CallTool(ctx, &CallToolParams{Name: "slow"}) | ||
| cancel() // cancel the tool call |
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.
Doesn't make sense. Add more code or at least "...".
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.
I added a full example.
|
This is awesome, BTW. |
Add a framework for feature documentation, and start populating it with
our SDK documentation.
This framework is as follows:
directory.
the top-level docs, supporting both linked code samples and generated
tables of contents.
examples, so that it is testable.
Some minor modifications to the weave tool were made to support this
framework.
Additionally, partially fill out this documentation with content on
base protocol and client features, as well as troubleshooting help.
Along the way, a bug was encountered that our LoggingTransport was not
concurrency safe. This is fixed with a mutex.
Fixes #466
Fixes #409
Updates #442