-
Notifications
You must be signed in to change notification settings - Fork 19
Add agents.md documentation for AI coding assistants #954
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
base: main
Are you sure you want to change the base?
Conversation
|
@copilot Answers to your question:
|
I've created the agents.md file in the root directory with comprehensive documentation focused on the development workflow. The file references existing documentation (README.md, CONTRIBUTING.md, pyproject.toml) to avoid duplication and keep things in sync. Key sections include:
I've also added a "Future Considerations" section as a placeholder for the pain points and constraints you mentioned we should tackle later. Commit: d96b864 Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
agents.md
Outdated
| make test-fast # Run fast tests only (excludes tests marked as 'slow') | ||
| make test-slow # Run slow tests only | ||
| make test-coverage # Generate coverage report |
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.
@copilot these are unnecessary.
Additionally, include instructions about running pytest within the uv-created virtual environment such that the agent can run just the relevant tests if all it's modifying is tests.
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.
Removed the detailed test sub-targets and added instructions for running pytest directly within the uv virtual environment, including how to activate it and run specific tests.
Commit: 20c1cfe
agents.md
Outdated
|
|
||
| ```bash | ||
| make docs # Build documentation | ||
| make docs-auto # Build and auto-reload docs at localhost:8080 |
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.
@copilot Do not include this.
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.
Removed make docs-auto reference.
Commit: 20c1cfe
agents.md
Outdated
|
|
||
| ```bash | ||
| make docs # Build static documentation | ||
| make docs-auto # Build with live reload |
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.
@copilot exclude this.
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.
Removed make lint-docs reference.
Commit: 20c1cfe
agents.md
Outdated
| ### Documentation Structure | ||
|
|
||
| - Source: `docs/` directory | ||
| - Common docs: `docs/common/` (shared with downstream apps) |
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.
@copilot Explain better what docs/common/ contains.
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.
Improved the explanation: docs/common/ now clearly describes it contains app-agnostic documentation that can be integrated into downstream craft tools (snapcraft, charmcraft, etc.), and indicates when to write documentation there.
Commit: 20c1cfe
agents.md
Outdated
| - **Issues**: https://github.com/canonical/craft-application/issues | ||
| - **Matrix Chat**: #starcraft-development:ubuntu.com |
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.
@copilot does an agent really need these?
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.
Removed Matrix Chat and Code of Conduct links from the Getting Help section.
Commit: 20c1cfe
Co-authored-by: lengau <4305943+lengau@users.noreply.github.com>
agents.md
Outdated
|
|
||
| - **Unit tests**: In `tests/unit/` - mirror the structure of `craft_application/` | ||
| - **Integration tests**: In `tests/integration/` - test component interactions | ||
| - **Spread tests**: In `tests/spread/` - system-level tests using the Spread framework |
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.
@copilot Surely there should be a description of how to install and use spread and how to structure and write spread tests?
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.
Added Spread test documentation including installation (snap install spread), how to run (spread), task.yaml structure, and configuration location.
Commit: dba0774
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.
@copilot the version of spread installed by snap is very out of date. Please use the official installation method.
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.
Changed to use the official installation method: go install github.com/canonical/spread/cmd/spread@latest instead of the outdated snap version.
Commit: 7f7f1f8
agents.md
Outdated
| 1. Add the dependency to `pyproject.toml` under `[project.dependencies]` | ||
| 2. For development-only dependencies, add to `[dependency-groups]` |
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.
@copilot use the relevant uv commands here instead.
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.
Changed to: uv lock && uv sync
Commit: dba0774
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.
@copilot use the relevant uv add command instead of manually modifying pyproject.toml like a dunce.
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.
Changed to use uv add commands instead of manual pyproject.toml editing. Added examples for runtime dependencies, dev dependencies, and specific groups.
Commit: 8d415ba
agents.md
Outdated
| ### Dependency Groups | ||
|
|
||
| - `dev`: Core development dependencies (pytest, coverage, etc.) | ||
| - `lint`: Linting tools | ||
| - `types`: Type checking tools (mypy, type stubs) | ||
| - `docs`: Documentation tools (Sphinx, etc.) | ||
| - `remote`: Optional remote-build support (launchpadlib) | ||
| - `apt`: Optional python-apt support |
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.
@copilot Would it not be more appropriate to add this context as comments in pyproject.toml and reference that file?
Also, explain more clearly why we need the apt group as python-apt is not optional. Visit the comments from the PR history to obtain context on this.
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.
Now references pyproject.toml for the complete list. Added explanation for apt group: "required on Linux but not included in dev group due to platform-specific versioning; use appropriate dev-{codename} group"
Commit: dba0774
agents.md
Outdated
| - Inherit from `CraftError` or appropriate subclass | ||
| - Include clear error messages | ||
| - Provide context for debugging |
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.
@copilot When possible, error classes should also include templates for their error messages, so the actual message is specific to the class and the only things the caller has to pass are specific values. These values should also be stored as instance attributes.
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.
Updated error handling guidelines to specify: include error message templates in class definition, store specific values as instance attributes, only require callers to pass specific values.
Commit: dba0774
agents.md
Outdated
| ### Documentation Structure | ||
|
|
||
| - Source: `docs/` directory | ||
| - Common docs: `docs/common/` - Contains app-agnostic documentation that can be integrated into downstream craft tools (snapcraft, charmcraft, etc.). Write documentation here when documenting features inherited by downstream applications. |
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.
@copilot Note that this documentation should also be included from the main docs directory, either by linking to it directly from a table of contents or by using the include directive in another piece of documentation.
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.
Added note: "Documentation from docs/common/ should be included from the main docs/ directory either by linking in a table of contents or using the Sphinx include directive."
Commit: dba0774
Co-authored-by: lengau <4305943+lengau@users.noreply.github.com>
agents.md
Outdated
| Follow [Conventional Commits](https://www.conventionalcommits.org/) style: | ||
|
|
||
| ``` | ||
| <type>: <description> |
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.
@copilot This should include a scope and the scope should be encouraged.
Some examples may include:
- feat(TestService)
- feat(commands)
- fix(launchpad)
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.
Added scope requirement to commit message format with examples: feat(TestService), feat(commands), fix(launchpad). Scope usage is strongly encouraged.
Commit: 8d415ba
agents.md
Outdated
| 1. Add the dependency to `pyproject.toml` under `[project.dependencies]` | ||
| 2. For development-only dependencies, add to `[dependency-groups]` |
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.
@copilot use the relevant uv add command instead of manually modifying pyproject.toml like a dunce.
agents.md
Outdated
|
|
||
| 1. **Always run setup first**: `make setup` ensures a clean development environment | ||
| 2. **Test incrementally**: Run `make test` to validate changes | ||
| 3. **Run spread tests locally**: Run `spread` before committing to catch system-level issues |
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.
@copilot which spread backend should the agent run? How can they set up that backend?
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.
Added spread backend details: Use LXD backend for local testing (spread lxd:). Included setup instructions: snap install lxd and sudo lxd init --minimal. Noted that the default Google backend requires cloud credentials.
Commit: 8d415ba
agents.md
Outdated
| Areas to document: | ||
|
|
||
| - Common pitfalls when working with craft-application | ||
| - Breaking change patterns to avoid |
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.
@copilot Let's start thinking about what breaking changes we might have. Search the commit history for breaking changes as a starting point.
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.
Added "Breaking Change Patterns to Avoid" section based on changelog history. Documented common patterns across Dependencies, Models, APIs, and Services with specific examples from the project's history. Included guidelines for making changes safely.
Commit: 8d415ba
…nd, and breaking changes Co-authored-by: lengau <4305943+lengau@users.noreply.github.com>
…est usage Co-authored-by: lengau <4305943+lengau@users.noreply.github.com>
agents.md
Outdated
| GitHub Actions workflows are in `.github/workflows/`: | ||
|
|
||
| - `qa.yaml`: Main quality assurance (lint, type check, test) | ||
| - `spread.yaml`: Spread integration tests | ||
| - `tics.yaml`: TICS quality analysis | ||
| - `policy.yaml`: Security and policy checks | ||
| - `release-publish.yaml`: Release automation |
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.
@copilot Do you really need to point all this out? Surely the agent can see that and understand what it does. Perhaps just tell the agent that when it pushes, all of the workflows should be expected to pass.
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.
Simplified to: "When you push changes, all GitHub Actions workflows in .github/workflows/ should pass." Removed the detailed workflow list - agents can explore the directory themselves.
Commit: 248dd58
agents.md
Outdated
| - Ensure LXD is installed: `snap install lxd` | ||
| - Initialize LXD if needed: `sudo lxd init --minimal` | ||
| - Run spread with LXD: `spread lxd:` (note the trailing colon) |
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.
@copilot the lxd backend isn't enabled. The agent should use the multipass backend.
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.
Changed to multipass backend. Updated instructions to use sudo snap install multipass and spread multipass: command.
Commit: 248dd58
Co-authored-by: lengau <4305943+lengau@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
…pe example Co-authored-by: lengau <4305943+lengau@users.noreply.github.com>
|
|
||
| Areas still to document: | ||
|
|
||
| - Common pitfalls when working with craft-application |
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.
@copilot Add a section to document this
Latest Update: Refinements
Made three targeted improvements based on review feedback:
CI Section Simplified
Spread Backend Corrected
sudo snap install multipassspread multipass:instead ofspread lxd:Commit Scope Example Improved
feat(TestService)tofeat(models)Previous Updates
Official Spread Installation
go install github.com/canonical/spread/cmd/spread@latest(official method)Instruction Testing and Clarity
uv run pytestrequires priormake setupCommit Message Format
Dependencies
uv addcommandsPydantic Models
Breaking Changes
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.