Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2148 +/- ##
=======================================
Coverage 73.86% 73.86%
=======================================
Files 227 227
Lines 37518 37518
=======================================
Hits 27712 27712
Misses 9806 9806 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
First, I love this. As I think about it though, I think this should be done as part of a broader review of our contribution policies.
For example, much of this feels like general contribution guidelines and may more appropriately belong in CONTRIBUTING.md. AGENTS.md could/should include a directive to read and adhere to those guidelines, AND we should make sure those guidelines are efficient (this is good for humans as much as LLMs)
I also really like how Zebra adds "compliance checks" at the beginning. This way an LLM will warn the user if their PR is likely to be reject. This saves the users time AND the maintainers time.
The latter point becomes much more effective when coupled with some of the policy changes like ZFND recently made; namely that PRs may/will be closed without review IFF there is no pre-existing issue with sufficient conversation history to justify beginning work. For this reason, I think this should be done as a broader review of our contribution policies.
| - Provide side-effect capabilities as explicit trait arguments (e.g., `clock: impl Clock`). | ||
| - Define capability traits in terms of domain types, not implementation details. | ||
|
|
||
| ## Architecture Patterns |
There was a problem hiding this comment.
I feel like a Crate Architecture section like what zebra/AGENST.md has would be very helpful context for an LLM
There was a problem hiding this comment.
We have this information in a Mermaid graph in the README; perhaps we should reference that?
| - Provide side-effect capabilities as explicit trait arguments (e.g., `clock: impl Clock`). | ||
| - Define capability traits in terms of domain types, not implementation details. | ||
|
|
||
| ## Architecture Patterns |
There was a problem hiding this comment.
We have this information in a Mermaid graph in the README; perhaps we should reference that?
| - Each commit that alters public API must also update docs and changelog in the same commit. | ||
| - Use `git revise` to maintain clean history within a PR. | ||
| - Commit messages: short title (<120 chars), body with motivation for the change. | ||
|
|
There was a problem hiding this comment.
We need to add a section on branching: merge-based workflow (no squash merges permitted), branch based on maintenance branches when making bug fixes to released code, we enforce strict Rust-Flavored SemVer compliance (we consider MSRV bumps to be SemVer breaking changes), and therefore we require that fixes be branched from the earliest existing maint/ branch for the crate in question so that we're able to make a semver compatible release with the fix. If no maint/ branch is available, the branch should be from the latest release tag from the crate in question at the Rust-flavored semver "breaking" (x.y.z or 0.x.y) version depended upon by the earliest maint/zcash_client_sqlite branch available.
There was a problem hiding this comment.
I'll add this, but it may be out of scope for an agent. Unless you want your agent branching and committing for you. I don't, and I don't want to imply that an agent should take these steps on my behalf, but the proof is in the pudding.
There was a problem hiding this comment.
What I want from this is for the agent to take branching & semver compatibility into consideration; basically, to prompt me to branch from (or rebase to) earlier when/if it can.
|
All valid concerns @nullcopy and @nuttycom. I've made some changes to try to address them 👍 . I don't want the scope to creep too much here though. This is a rather large AGENTS.md file (I think typically they're ~150 lines). We just need to get something in place as a starting point and then we'll find out what's working and what's not as we get our work done. |
|
I opened a similar PR on zcash/wallet; we should consider (a) reproducing some of the changes I made to that CONTRIBUTING.md here, and (b) unify our approach to AGENTS.md: zcash/wallet#377 |
Thes adds an initial AGENTS.md file (for humans and others).