Copilot improvements #5111
Copilot improvements #5111
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request makes three key improvements to the stellar-core development environment and tooling: updates the devcontainer configuration for Ubuntu 24.04 compatibility, restructures the Copilot instructions to emphasize using tools and skills, and introduces eight comprehensive skills for code development, review, testing, and validation workflows.
Changes:
- Updated devcontainer configuration from Ubuntu 20.04 to 24.04 (noble) with modern VSCode customizations and development tool extensions
- Rewrote Copilot instructions to guide agents toward using LSP tools, subagents, and skill-based workflows rather than directly providing coding guidelines
- Added eight new skill documents covering validation, testing, building, code review (high and low level), test addition, build configuration, and skill creation
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| .github/copilot-instructions.md | Rewrites instructions to emphasize agent-based workflows and tool usage instead of direct coding guidelines |
| .devcontainer/devcontainer.json | Updates container name to Ubuntu 24.04, modernizes customizations structure, and adds development extensions |
| .devcontainer/Dockerfile | Updates for Ubuntu 24.04 sources, adds user/group handling logic, and installs additional development tools |
| .claude/skills/validating-a-change/SKILL.md | Defines comprehensive validation workflow orchestrating multiple review and testing steps |
| .claude/skills/running-tests/SKILL.md | Provides detailed test execution guidance from smoke tests through sanitizer builds |
| .claude/skills/running-make-to-build/SKILL.md | Documents build system structure and correct usage patterns |
| .claude/skills/low-level-code-review/SKILL.md | Establishes mechanical code review process for localized issues |
| .claude/skills/high-level-code-review/SKILL.md | Defines semantic code review criteria for correctness and design consistency |
| .claude/skills/configuring-the-build/SKILL.md | Documents build configuration options and autotools workflow |
| .claude/skills/adding-tests/SKILL.md | Guides test analysis and creation for various change types |
| .claude/skills/adding-a-new-skill/SKILL.md | Provides meta-framework for creating additional skills |
graydon
force-pushed
the
copilot-improvements
branch
from
ebea2b8 to
29a76d8
Compare
graydon
force-pushed
the
copilot-improvements
branch
from
29a76d8 to
9c2a077
Compare
|
|
||
| Example: | ||
| ```bash | ||
| ./stellar-core test --ll fatal -r simple --disable-dots --abort "test name" |
There was a problem hiding this comment.
Did you have success with agents running tests consistently using this skill? I wonder if a short version of this should belong to instructions for better discoverability.
There was a problem hiding this comment.
I would not say "consistently", no. I'll put it in instructions.
|
|
||
| # Overview | ||
|
|
||
| The build works like this: |
There was a problem hiding this comment.
I wonder if we need to be so verbose here. Wouldn't just specifying a common command line work? I also wonder if that command line could just go to instructions. I'm fairly skeptical of a need to do something complex with the build, using a single fixed command line would probably be sufficient for local development, and re-configuring builds is generally rare.
There was a problem hiding this comment.
I agree makes sense to hoist the "most likely" configure / make / run-one-test / make check in parallel commands up to the instructions file. I figured I'd tell them some details on other parts of the build system since they're not uncommon in my experience -- I switch compilers or configure flags fairly often -- but there's no need for them to dig into such a file the majority of the time. I'll make this adjustment.
(Also it seems like I was a bit too optimistic about the ability of subagents to execute complex steps on their own, they have a tendency to just wander off; I will alter the language around them)
Co-authored-by: Leigh <[email protected]>
3 participants
This PR does 3 main things:
I don't think there's much to object to in the skills and I am also not all that certain that copilot will use them as actively as we'd like; on the other hand it roughly seems to work with my local testing and enables some behaviors that I was otherwise telling it over and over / watching it do wrong. so I think it's probably worth starting off with something like this. if we find them making things worse they can always be edited back down / removed.