From e0e530e646c4cdb05a2ea0cac8534d6e5fbb6afc Mon Sep 17 00:00:00 2001 From: "firat.sertgoz" Date: Sun, 29 Mar 2026 09:11:58 +0300 Subject: [PATCH] docs: tighten contribution and PR guidance (#1704) --- .github/pull_request_template.md | 17 ++++--- CONTRIBUTING.md | 77 +++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 6 deletions(-) diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md index 4fc7cbf2..e6fe6128 100644 --- a/.github/pull_request_template.md +++ b/.github/pull_request_template.md @@ -6,7 +6,7 @@ ## Change Type - + - [ ] Bug fix - [ ] New feature @@ -18,16 +18,19 @@ ## Linked Issue - + ## Validation -- [ ] `cargo fmt` -- [ ] `cargo clippy --all --benches --tests --examples --all-features` +- [ ] `cargo fmt --all -- --check` +- [ ] `cargo clippy --all --benches --tests --examples --all-features -- -D warnings` +- [ ] `cargo build` - [ ] Relevant tests pass: +- [ ] `cargo test --features integration` if database-backed or integration behavior changed - [ ] Manual testing: +- [ ] If a coding agent was used and supports it, `review-pr` or `pr-shepherd --fix` was run before requesting review ## Security Impact @@ -45,6 +48,10 @@ +## Review Follow-Through + + + --- -**Review track**: +**Review track**: diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1c5c6d88..c7a2b2df 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,6 +10,42 @@ cd ironclaw This installs the Rust toolchain, WASM targets, git hooks, and runs initial checks. +## How to Contribute + +- Bug fixes, docs improvements, and focused cleanup tied to a concrete problem are welcome. +- Search existing issues and PRs before opening a new one to avoid duplicates. +- Keep changes scoped. One bug, one feature, or one documentation improvement per PR. + +### Creating Issues + +Open an issue when you are reporting a bug, proposing a feature, or documenting a gap in behavior. + +For bug reports, include: + +- What you expected to happen +- What actually happened +- Clear reproduction steps +- Relevant logs, screenshots, or error output +- Environment details when they matter (OS, database backend, feature flags, commit/branch) + +For feature requests: + +- Open an issue first before writing code +- Explain the problem being solved, not just the implementation idea +- Wait for maintainer feedback before investing in a large PR + +We require an issue for new features so maintainers can prioritize the work and confirm it fits the roadmap before anyone spends time implementing it. + +### Fixing Bugs + +- Small, targeted bug-fix PRs are welcome +- If there is already an issue, link it in your PR +- If the bug is non-trivial, security-sensitive, or changes behavior across subsystems, open or confirm an issue first so the approach can be aligned before implementation + +### Refactor-Only PRs + +Refactor-only PRs are not accepted from contributors outside the core team. If a refactor is necessary to land a bug fix or approved feature, keep it minimal and clearly tied to that change. + ## Development Workflow ```bash @@ -19,6 +55,45 @@ cargo test # unit tests cargo test --features integration # + PostgreSQL tests ``` +These commands are for day-to-day iteration while you are developing locally. The pre-submission checks below are intentionally stricter and use CI-style flags so you can catch formatting drift and clippy warnings before requesting review. + +## Before You Open a PR + +Run the local validation checks required before requesting a review. These are stricter than the commands for iterative development: + +```bash +cargo fmt --all -- --check +cargo clippy --all --benches --tests --examples --all-features -- -D warnings +cargo build +cargo test +``` + +Also run this when your change touches database-backed or integration behavior: + +```bash +cargo test --features integration +``` + +Before asking for review: + +- Build and exercise the changed path locally, not just the narrowest unit test +- Keep the PR focused and avoid mixing unrelated concerns +- Fill out the PR template with a clear summary, validation notes, and impact assessment +- If your change affects tracked behavior, update `FEATURE_PARITY.md` in the same branch +- If onboarding or setup behavior changes, update the relevant setup docs in the same branch +- If you are using a coding agent and it supports them, run `review-pr` or `pr-shepherd --fix` before opening or updating the PR +- `codex review --base origin/main` is also encouraged before requesting review + +## Review Follow-Through + +Review conversations are author-owned. + +- Address each review comment with a code change or a clear explanation +- Resolve conversations you have handled; leave them open only when reviewer judgment is still needed +- Do not leave review cleanup for maintainers when the follow-through belongs to the author + +If a PR is stale for more than 48 hours after review feedback is posted, maintainers may take over the follow-up work and land the changes needed to accomplish the original PR or issue intent. + ## Code Style - Zero clippy warnings policy @@ -46,7 +121,7 @@ All PRs follow a risk-based review process: | Track | Scope | Requirements | |-------|-------|-------------| | **A** | Docs, tests, chore, dependency bumps | 1 approval + CI green | -| **B** | Features, refactors, new tools/channels | 1 approval + CI green + test evidence | +| **B** | Features, maintainer-requested refactors, new tools/channels | 1 approval + CI green + test evidence | | **C** | Security (`src/safety/`, `src/secrets/`), runtime (`src/agent/`, `src/worker/`), database schema, CI workflows | 2 approvals + rollback plan documented | Select the appropriate track in the PR template based on what your changes touch.