-
Notifications
You must be signed in to change notification settings - Fork 320
Create a TSP file starter document #3351
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
base: main
Are you sure you want to change the base?
Create a TSP file starter document #3351
Conversation
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.
Pull Request Overview
This PR adds comprehensive documentation for creating TypeSpec-based Rust SDK clients. It introduces a detailed step-by-step guide covering the full workflow from TypeSpec setup through code generation, testing, and CI/CD configuration.
Key changes:
- New 900+ line guide for creating TypeSpec-based Rust SDK clients with both fully-generated and partially-generated approaches
- GitHub Copilot prompt template to help contributors generate SDK crates from TypeSpec definitions
- Minor formatting fix to existing perf-test prompt file
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| doc/new-typespec-based-client.md | Comprehensive guide covering TypeSpec setup, code generation, testing, documentation, and CI/CD for new Rust SDK clients |
| .github/prompts/typespec-client.mc | Agent prompt template providing quickstart instructions for TypeSpec-to-Rust SDK generation workflow |
| .github/prompts/perf-test.prompt.md | Adds missing prompt code fence markers for consistency |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
ronniegeraghty
left a comment
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.
Nothing blocking, except maybe the wrong file extension on the prompt file.
But mostly just nits and recommendations on additions that service teams may find helpful.
DOH! I wonder if that might have been why some of the copilots struggled. |
heaths
left a comment
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 think there was a lot generated that doesn't apply to this repo and needs to be reviewed carefully.
heaths
left a comment
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 don't hate it, but I still think it's too long because of duplicated content when we could just refer to files - even by path - which even an LLM can follow just fine especially if we make them links anyway. I think describing the steps is great, but not always the file content near-copies.
| ### Step 4.6: Build and Format | ||
|
|
||
| Same as Step 3.2. | ||
| For an example of a partially modified client, look at the [Azure KeyVault Certificates](https://github.com/Azure/azure-sdk-for-rust/tree/main/sdk/keyvault/azure_security_keyvault_certificates). |
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.
Could also link to the guidelines. In fact, we should include a link to the guidelines somewhere herein anyway. They should really review at least the initial bits.
| ### Step 6.1: Create README.md | ||
| Create `sdk/<service-dir>/<crate-name>/README.md`: |
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.
Couldn't we just tell people to look at other README.md files like you do elsewhere here? Based on other languages, I think we just always refer to KV secrets except in the case of the custom client example with KV certs. Or maybe we just use KV certs as a reference everywhere herein because of that. Duplicating this information not over creates cognitive overload but makes maintenance harder because we have to remember to update this as well if we ever change anything.
Fixes #1882
@ronniegeraghty for detailed commentary.
Largely created by copilot with substantive edits after the fact.
Copilot prompt tested successfully on "chatgpt 5 Codex", mostly successfully on "Claude Sonnet 3.5"