Rewrite Copilot instructions and move to AGENTS.md#53630
Rewrite Copilot instructions and move to AGENTS.md#53630lbussell wants to merge 11 commits intodotnet:mainfrom
Conversation
nagilson
left a comment
nagilson
left a comment
There was a problem hiding this comment.
Thanks, I think that it's a smart idea to improve the instructions we have nowadays! Left some questions to ponder.
|
|
||
| ### Building | ||
|
|
||
| ```bash |
There was a problem hiding this comment.
might this confuse the agent since bash is more unixy?
There was a problem hiding this comment.
This issue came back with the new edits. Your agent seems to like assuming bash 😁
|
|
||
| This puts the built SDK on your PATH so `dotnet build`, `dotnet test`, etc. use your local changes. | ||
|
|
||
| ### Running Tests |
There was a problem hiding this comment.
One use case that I have encountered a number of times recently is simulating the Helix run environment. This is described in https://github.com/dotnet/sdk/blob/main/documentation/project-docs/repro-helix-failure.md. Scenarios in which this is important is to flush out dependencies on test-assets where the available content is limited. Thoughts on if this scenario should be called out?
There was a problem hiding this comment.
Is that document up-to-date? If so I can link to it here.
|
|
||
| By default (without `-pack`), the build skips crossgen and installers (`SkipUsingCrossgen=true`, `SkipBuildingInstallers=true`) to speed up inner-loop iteration. Build output goes to `artifacts/bin/redist/<Configuration>/dotnet/`. | ||
|
|
||
| Key flags: |
There was a problem hiding this comment.
There are some secondary flags that can be important. e.g. BuildSdkDeb, BuildSdkRpm. I am sure there are others. Is it worth noting these or giving hints on how to self discover these?
There was a problem hiding this comment.
How do you discover these? Are they documented somewhere centrally? Or do you just need to inspect some of the msbuild files in the repo?
There was a problem hiding this comment.
I don't know of them being called out in any docs. I always look at the build definition to see what build options/parameters it uses. For the ones I called out, they are specified here - https://github.com/dotnet/sdk/blob/main/eng/pipelines/templates/jobs/sdk-job-matrix.yml#L26
There was a problem hiding this comment.
Feel free to propose more specific changes if you know what you want to see here.
lbussell
force-pushed
the
improve-instructions
branch
from
f3f0691 to
9cac85c
Compare
There was a problem hiding this comment.
Pull request overview
Updates repository-level agent guidance by replacing the previous Copilot instructions file with a new AGENTS.md focused on repo orientation and local build/test workflow.
Changes:
- Add
AGENTS.mdwith repo overview, build/test commands, and a high-level project layout map. - Remove
.github/copilot-instructions.md.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| AGENTS.md | Introduces new repo overview + local workflow guidance intended for agent use. |
| .github/copilot-instructions.md | Removes the prior Copilot instruction content. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
/ba-g docs-only change. |
nagilson
left a comment
nagilson
left a comment
There was a problem hiding this comment.
I like the idea to move to AGENTS.md! Have you tried comparing the results of llms with the old instructions vs the new instructions? I'm curious if it changes the behavior at all with how successful they are running tests, editing the code, etc, in the scenarios outlined by this and the prior doc (e.g. in eng/common, with mstest vs xunit tests, whether it still uses skip parameter and includes a github issue when skipping a test), when making a large change if it still respects the style of the file it's in?
| @@ -1 +0,0 @@ | |||
| D:/code/dotnet-sdk/.claude/skills/ No newline at end of file | |||
| This is the .NET SDK repo. | ||
| It contains the `dotnet` CLI, MSBuild tasks/targets, project system SDKs, template engine, workload management, and related tooling. |
There was a problem hiding this comment.
| This is the .NET SDK repo. | |
| It contains the `dotnet` CLI, MSBuild tasks/targets, project system SDKs, template engine, workload management, and related tooling. | |
| The .NET SDK contains the `dotnet` CLI, MSBuild tasks/targets, project system SDKs, template engine, workload management, and related tooling. |
nit:
Does the agent need to be told this is the sdk repo? Did that help in any way?
|
|
||
| This is the .NET SDK repo. | ||
| It contains the `dotnet` CLI, MSBuild tasks/targets, project system SDKs, template engine, workload management, and related tooling. | ||
| This repo uses https://github.com/dotnet/arcade for build, test, and pipeline infrastructure. |
There was a problem hiding this comment.
nit: technically I think 'repo' refers to the digital source storage mechanism, aka 'https://github.com/dotnet/sdk' but this source code folder is not a 'repo' if it is cloned. I don't think this is the best terminology. We could be more concise as well using bullet points instead of full sentences with 'this repo' 'the sdk'
| ### Test | ||
|
|
||
| Do not run the full test suite locally, it takes hours. | ||
| Instead, run only the tests relevant to your changes. |
There was a problem hiding this comment.
| Instead, run only the tests relevant to your changes. | |
| Instead: build, then run only relevant tests, like so: |
There was a problem hiding this comment.
'your' confuses context with 3rd person vs first person
| ```bash | ||
| # Run all tests in a specific test project | ||
| ./.dotnet/dotnet test test/<ProjectName>.Tests | ||
|
|
There was a problem hiding this comment.
should we remove the mstest vs xunit bifurcation guide?
| # Run a single test by class | ||
| ./.dotnet/dotnet test test/<ProjectName>.Tests --filter "ClassName=Microsoft.DotNet.Cli.SomeTests" | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Do you think this is helpful and we should keep this?
Do not manually edit files under documentation/manpages/sdk as these are generated based on documentation and should not be manually modified.
| | `dotnet` CLI (entry point & commands) | `src/Cli/` | | ||
| | Template content | `template_feed/` | | ||
| | Build infrastructure | `eng/` | | ||
| | Arcade infrastructure (do not edit) | `eng/common` | |
There was a problem hiding this comment.
Does the do not edit warning here make agents worse at modifying props / targets for codeflow ?
There was a problem hiding this comment.
I opened dotnet/arcade#16757 which is a more general solution to keeping agents from messing up eng/common.
The bulk of
.github/copilot-instructions.mdwas written before most frontier models were released. In my limited experience working with this repo, the existing instructions were mostly a waste of context/tokens and sometimes even reduced the quality of Copilot's output.I have rewritten the instructions based on Write effective instructions | Claude Code documentation.
Before: ~700 tokens
After: ~500 tokens
The new instructions focus on telling agents the basics of how to work in the repo, and avoid overloading them with instructions. Research shows that LLMs' ability to follow instructions degrades the more rules that they are given (see: How Many Instructions Can LLMs Follow at Once?).