-
Notifications
You must be signed in to change notification settings - Fork 545
✨ Set up Copilot instructions for repository #858
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
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
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.
These instructions seem to be focusing on project structure, somewhat superficially. It might make sense for us to capture notes on less obvious aspects of our architecture and test structure, but I suspect this is going to require a manual write-up.
@copilot, please address the feedback. Note that instructions should be more focused on implementing the library than they are on consuming the library. |
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Addressed in commit c4db428. The instructions now focus on implementing the library with sections on:
|
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.
We should include a session describing the key types, architectural strata and their relations that the library defines. Examples include:
- The DTO types in the
Protocol
folder - The built-in JSON-RPC implementation,
- The
ITransport
abstraction, - The
McpSession
,McpServer
, andMcpClient
types, - The two transport implementations provided and their invariants,
- Details on our approach to serialization, e.g. how JSON-RPC message serialization hardcode to the source generator whereas user defined types in tool calls must accept a
JsonSerializerOptions
.
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Added comprehensive section in commit a54c6c2 covering:
|
@copilot, a validation leg is failing:
Also, on some other PRs with copilot, it's seemingly been lazy about always building and running tests and ensuring that everything has successfully built and all tests passed before declaring success. Do we need anything in the instructions that is very explicit about that? Or should that be happening anyway? |
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
@copilot, on some other PRs with copilot, it's seemingly been lazy about always building and running tests and ensuring that everything has successfully built and all tests passed before declaring success. Do we need anything in the instructions that is very explicit about that? Or should that be happening anyway? |
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
Added explicit guidance in commit 7ba4a5f. The instructions now include:
|
This PR adds comprehensive GitHub Copilot instructions to help contributors and Copilot generate code that aligns with the repository's conventions and best practices, with a focus on library implementation rather than consumption.
What's Changed
Added
.github/copilot-instructions.md
with detailed guidance covering:Protocol/
folderBenefits
This file will help GitHub Copilot:
The instructions are based on actual conventions found in the codebase, including
.editorconfig
,Directory.Build.props
, existing implementation patterns insrc/
, test infrastructure intests/Common/
, and architectural patterns throughout the SDK.Fixes #780
Original prompt
Fixes #780
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.