add copilot instructions #3963
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| # Copilot Instructions for Pull Request Review | ||
|
|
||
| ## Purpose | ||
| These instructions guide Copilot and contributors in reviewing pull requests for the AWS .NET SDK library. The goal is to maintain high code quality, ensure backward compatibility, and follow best practices for library development. | ||
|
|
||
| ## Review Checklist | ||
|
|
||
| ### 1. API Compatibility | ||
| - Ensure **no breaking changes** are introduced in public APIs. | ||
| - All public methods, classes, and interfaces must remain backward compatible unless a major version bump is justified. | ||
|
|
||
| ### 2. Behavioral Changes | ||
| - Check for **changes in behavior** (e.g., logic, side effects, error handling). | ||
| - If behavior changes, ensure it is documented and justified in the PR description. | ||
|
|
||
| ### 3. Dev Config and Versioning | ||
| - Every PR must include a **dev config file** with the correct patch, minor, or major notation. | ||
| - The dev config should accurately reflect the scope of the change: | ||
| - **Patch**: Bug fixes, no API or behavior change. | ||
| - **Minor**: New features, backward compatible. | ||
| - **Major**: Breaking changes (should be rare). | ||
|
|
||
| ### 4. Platform Compatibility | ||
| - Ensure changes do not break support for the following .NET versions: | ||
| - .NET Standard 2.0 | ||
| - .NET Core 3.1 | ||
| - .NET 8.0 | ||
| - .NET Framework 4.7.2 | ||
|
|
||
| ### 5. General Best Practices | ||
| - Follow .NET and AWS SDK coding standards. | ||
|
There was a problem hiding this comment. What is it going to do with this when we don't have a coding standards doc or even memory bank info for this yet? It may be better to tell it to maintain coding consistency at this point since there are no standard docs. There was a problem hiding this comment. good question. So this is assuming that copilot has some already pretrained info on .NET and AWS SDK coding practices. I'm also assuming that during PR review it has context of the files and it can infer existing best practices as well. you are right though that having a dedicated coding standards doc would help more though. let me know your thoughts. |
||
| - Avoid introducing unused code, dependencies, or files. | ||
| - Ensure all code is properly formatted and linted. | ||
|
|
||
| ### 6. Generator Changes | ||
|
There was a problem hiding this comment. Is it going to understand you are referring to our generator (Doc or code)? I have a feeling it doesn't even know where that is at. There was a problem hiding this comment. So this is just based off of existing comments it gave e.g #3949 (review) it seems to know what it is and i believe it can infer (e.g. if file in the generator folder is modified then it refers to the generator) |
||
| - When replacing handwritten code with generated code, ensure the generated code is functionally equivalent to the original. | ||
| - Document any differences, edge cases, or limitations in the PR description. | ||
| - If possible, provide a comparison or summary of key behaviors between the old and new implementations. | ||
|
|
||
| ### 7. General Review Practices | ||
|
There was a problem hiding this comment. Add look for security vulnerabilities |
||
| - Review code for bugs, flaws in logic, and potential edge cases. | ||
| - Check for correct error handling and input validation. | ||
| - Ensure code is maintainable, readable, and follows best practices. | ||
| - Look for opportunities to simplify or optimize code where appropriate. | ||
|
|
||
| ## Automated Checks | ||
| - Copilot should flag PRs that: | ||
| - Introduce breaking changes without a major version bump. | ||
| - Lack a dev config file or changelog update. | ||
|
There was a problem hiding this comment. changelog updates come through the devconfig files so it may not understand this is where it needs to see it. Again stressing it would be useful to build out the devconfig section above just a tad more. |
||
| - Change public APIs without documentation or tests. | ||
| - Have failing unit or integration tests. | ||
| - Reduce test coverage for affected code. | ||
| - Introduce unhandled exceptions, missing error handling, or poor input validation. | ||
| - Increase code complexity or reduce maintainability. | ||
| - Do not document or justify changes in logic, edge cases, or optimizations. | ||
| - For changes related to migrating custom code to generated code, where evidence of equivalence (e.g., missing test results, behavioral comparison, or documentation) is not provided. | ||
|
|
||
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 think it is even going to begin to understand what this is. You likely need to obtain some information about it since we don't have the memory-bank yet but agree it cannot be huge for this purpose.
https://github.com/aws/aws-sdk-net/blob/boblod-ai-memory-bank-v2/memory-bank/partitions/sdkdeveloper/devConfig.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.
Perhaps the location and a sample of a devconfig file would be enough to start with the possible names.
Uh oh!
There was an error while loading. Please reload this page.
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 have this other PR #3968 which has info on dev config and contributing. maybe i can referencing that CONTRIBUTING.MD file?
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.
Yes, that should work. Otherwise it will have no clue what it is.