Skip to content

add copilot instructions #3963

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

Open
wants to merge 4 commits into
base: development
Choose a base branch
from
Open

add copilot instructions #3963

wants to merge 4 commits into from

Conversation

GarrettBeatty
Copy link
Contributor

@GarrettBeatty GarrettBeatty commented Aug 19, 2025

Add copilot custom instructions for copilot review. Customizing this file allows us to specifiy what copilot should look for during reviews

See: https://github.blog/changelog/2025-06-13-copilot-code-review-customization-for-all/

Motivation and Context

To improve copilot reviews

Testing

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

@GarrettBeatty GarrettBeatty marked this pull request as ready for review August 19, 2025 02:24
@GarrettBeatty GarrettBeatty requested a review from Copilot August 19, 2025 02:24
Copy link
Contributor

@Copilot Copilot AI left a 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 custom Copilot instructions to guide automated code review for the AWS .NET SDK library. The instructions focus on maintaining backward compatibility, proper versioning, and following best practices.

  • Establishes review guidelines for API compatibility, behavioral changes, and platform support
  • Defines requirements for dev config files and version bump notations
  • Outlines specific checks for generator code changes and automated flagging criteria

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

- 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
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor Author

@GarrettBeatty GarrettBeatty Aug 20, 2025

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?

Copy link
Contributor

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.

- .NET Framework 4.7.2

### 5. General Best Practices
- Follow .NET and AWS SDK coding standards.
Copy link
Contributor

@boblodgett boblodgett Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
Copy link
Contributor

@boblodgett boblodgett Aug 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

- 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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add look for security vulnerabilities

## Automated Checks
- Copilot should flag PRs that:
- Introduce breaking changes without a major version bump.
- Lack a dev config file or changelog update.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants