Enhance copilot-instructions.md with comprehensive repository onboarding and standard dotnet command usage - #11191
Conversation
…rding information Co-authored-by: radical <1472+radical@users.noreply.github.com>
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
There was a problem hiding this comment.
Pull Request Overview
This PR significantly enhances the .github/copilot-instructions.md file to provide comprehensive onboarding information for coding agents working on the Aspire repository. The enhancement transforms basic coding guidelines into a complete repository guide that includes repository overview, build prerequisites, testing infrastructure, and project architecture.
Key Changes
- Added comprehensive repository overview covering Aspire framework components and technology stack
- Enhanced build instructions with prerequisites, timing estimates, and troubleshooting guidance
- Expanded testing section with critical quarantined test exclusion requirements and validation commands
- Added detailed project architecture section covering directory structure, configuration files, and CI/CD workflows
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
|
||
| * Follow the instructions in the repo to build. | ||
| * If temporarily introducing warnings during a refactoring, you can add the flag `/p:TreatWarningsAsErrors=false` to your build command to prevent the build from failing. However before you finish your work, you must strive to fix any warnings as well. | ||
| **CRITICAL: Always use the local dotnet installation.** In building and testing, never use the global `dotnet` CLI. Use `./dotnet.sh` on Unix, `.\dotnet.cmd` on Windows. |
There was a problem hiding this comment.
Now that we have https://github.com/dotnet/aspire/blob/7ef1702e1d84a52ac221704b17fe3a492360fc7f/global.json#L6-L9 this isn't quite true.
There was a problem hiding this comment.
In fact is it possible that we can get rid of dotnet.sh/dotnet.cmd? I find that it confuses the agents more than helps them?
There was a problem hiding this comment.
Updated the instructions to reflect the current global.json configuration. Removed the strict requirement for dotnet.sh/dotnet.cmd and updated all examples to use standard dotnet commands, which will automatically use the local SDK when available after restore. This should reduce confusion for agents while maintaining the local SDK preference. (commit: updated)
There was a problem hiding this comment.
Copilot revert this change. We need to use the dotnet scripts to ensure that we are always using the expected versions, and they are installed.
There was a problem hiding this comment.
Reverted the changes as requested. The instructions now emphasize using ./dotnet.sh/./dotnet.cmd scripts instead of standard dotnet commands to ensure expected SDK versions are used. All test command examples have been updated accordingly. (commit: 4849130)
There was a problem hiding this comment.
Now that we have
Lines 6 to 9 in 7ef1702
"paths": [
".dotnet",
"$host$"
],
this isn't quite true.
Oh, I misunderstood this!
There was a problem hiding this comment.
Updated the instructions to use standard dotnet commands instead of the wrapper scripts. With the paths configuration in global.json, the standard dotnet command will automatically use the local SDK after restore. All test examples have been updated accordingly. (commit: 7f57f36)
There was a problem hiding this comment.
I think there is still a scenario were this doesn't work, which is when the user already has the require SDK globally installed. In that case, our build scripts won't restore the sdk and instead just use the globally defined one, so paths in global.json won't work. These instructions I believe also get used for folks pulling the repo locally when using Copilot, so there could be cases where folks run into this. For this reason, I'd still suggest to continue having our dotnet.sh/cmd scripts which account for this case.
There was a problem hiding this comment.
I agree that we shouldn't drop the scripts as it would still be useful to have the it restore the necessary bits.
These instructions I believe also get used for folks pulling the repo locally when using Copilot, so there could be cases where folks run into this.
Can you elaborate on this one? Would you be able to try this out and see if it breaks because the copilot instructions here?
Co-authored-by: radical <1472+radical@users.noreply.github.com>
Co-authored-by: radical <1472+radical@users.noreply.github.com>
Co-authored-by: radical <1472+radical@users.noreply.github.com>
| **Always run restore first to set up the local SDK.** Run `./restore.sh` (Linux/macOS) or `./restore.cmd` (Windows) first to install the local SDK. After restore, you can use standard `dotnet` commands, which will automatically use the local SDK when available due to the paths configuration in global.json. | ||
|
|
||
| #### Prerequisites | ||
| 1. **Container Runtime Required**: Install Docker Desktop or Podman before building/testing |
There was a problem hiding this comment.
Drop this. We only run the container based tests on Linux where Docker is already installed.
There was a problem hiding this comment.
Copilot Drop this. We only run the container based tests on Linux where Docker is already installed.
There was a problem hiding this comment.
Removed the container runtime prerequisite and related references since container-based tests only run on Linux where Docker is already installed. (commit: 3dbdb00)
…on Linux where Docker is pre-installed Co-authored-by: radical <1472+radical@users.noreply.github.com>
| dotnet.sh test tests/Aspire.Hosting.Testing.Tests/Aspire.Hosting.Testing.Tests.csproj --logger "console;verbosity=detailed" -- --filter-method "TestingBuilderHasAllPropertiesFromRealBuilder" | ||
| (4) To run specific tests, include the filter after `--`: | ||
| ```bash | ||
| dotnet test tests/Aspire.Hosting.Testing.Tests/Aspire.Hosting.Testing.Tests.csproj -- --filter-method "TestingBuilderHasAllPropertiesFromRealBuilder" --filter-not-trait "quarantined=true" |
There was a problem hiding this comment.
is it clear enough that they should be using dotnet.sh from the root? there is a global dotnet installed on these machines (for MCP servers) -- they should not use that.
There was a problem hiding this comment.
IIUC the assumption here is that we have at least dotnet 10.x installed which supports the paths in global.json, which should then pick up $repo/.dotnet/dotnet, and thus still be using the restored dotnet.
There was a problem hiding this comment.
Does paths have a fallback for the globally installed dotnet in case it is already present? In that case, our build won't have it under the .dotnet folder so we should make sure that works.
Dan Moseley (danmoseley)
left a comment
There was a problem hiding this comment.
Looks good, philosophically it's funny that this is all applicable to human contributors (albeit Linux only). I wonder whether our contributing doc should mostly point to this. conversely, this doc could point to that. fine for now but I bet we'll end up doing one or the other.
// human-instructions.md
see: copilot-instructions.md :) |
This PR significantly expands the
.github/copilot-instructions.mdfile to provide comprehensive onboarding information for coding agents working on the Aspire repository for the first time, with updated guidance to use standarddotnetcommands that leverage the global.json paths configuration.Key Changes
Repository Overview
Updated Build and Test Instructions
dotnetcommands that automatically use the local SDK after restore due to global.json paths configurationTesting Infrastructure
--filter-not-trait "quarantined=true"dotnetcommands for consistencyProject Architecture
/src,/tests,/playground,/docs,/engglobal.json,.editorconfig,Directory.Build.propsThe updated instructions leverage the global.json paths configuration that automatically uses the local SDK after restore, eliminating the need for wrapper scripts while maintaining SDK version consistency. All documented commands have been tested and verified to work with the current repository configuration.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.