From 8c6993b36e999fdbace0009b9e4b0a12d8dd7e11 Mon Sep 17 00:00:00 2001 From: ian-flores Date: Mon, 2 Mar 2026 12:19:41 -0800 Subject: [PATCH] docs: add PR self-review norms and code flow section --- .claude/review-guidelines.md | 23 +++++++++++++++++++++ .github/pull_request_template.md | 34 ++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) create mode 100644 .github/pull_request_template.md diff --git a/.claude/review-guidelines.md b/.claude/review-guidelines.md index caeb094..c2960c6 100644 --- a/.claude/review-guidelines.md +++ b/.claude/review-guidelines.md @@ -1,5 +1,13 @@ # Team Operator Code Review Guidelines +## Reviewer Responsibilities + +Your job is to review the **code itself**, not just the description or intent. In a world of agentic coding, the author may not have hand-written every line. That means: + +- Read the actual diff, not just the PR summary +- Verify the code does what the description claims +- Look for correctness issues, not just style preferences + ## Core Principles ### Simplicity @@ -14,6 +22,12 @@ - Dependencies should be minimal and justified - Breaking changes need migration paths +### Code Quality Fundamentals +- **Encapsulation**: Is internal state properly hidden? Are interfaces clean? +- **DRY**: Is there duplicated logic that should be extracted? But don't over-abstract — three similar lines can be better than a premature helper +- **Naming**: Do names reveal intent? Would a future reader understand this? +- **Complexity**: Is this more complicated than it needs to be? + ### Security (Elevated Scrutiny) These changes require extra review attention: @@ -60,3 +74,12 @@ Use clear, actionable language: - **Critical**: "This will break X because Y. Consider Z." - **Important**: "This pattern differs from existing code in A. Recommend B for consistency." - **Suggestion**: "Consider X for improved Y." + +## Self-Review Norm + +PR authors are expected to review their own diff before opening the PR and add inline comments to draw attention to: +- Lines they don't fully understand +- Areas of concern or uncertainty +- Key decision points reviewers should weigh in on + +This balances effort between author and reviewer. If the author hasn't commented their PR, ask them to. diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md new file mode 100644 index 0000000..6e8d65e --- /dev/null +++ b/.github/pull_request_template.md @@ -0,0 +1,34 @@ +# Description + + + +## Code Flow + + + +## Category of change + +- [ ] Bug fix (non-breaking change which fixes an issue) +- [ ] New feature (non-breaking change which adds functionality) +- [ ] Build: a code change that affects the build system or external dependencies +- [ ] Performance: a code change that improves performance +- [ ] Refactor: a code change that neither fixes a bug nor adds a feature +- [ ] Documentation: documentation changes +- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) + +## Checklist + +- [ ] I have run `just test` and all tests pass +- [ ] I have reviewed my own diff and added inline comments on lines I want reviewers to focus on or that I am uncertain about