|
| 1 | +--- |
| 2 | +name: final-review |
| 3 | +description: Performs comprehensive final review of pop-cli code changes before commit or PR. Use when finishing implementation, preparing to commit, creating a PR, or when asked to review changes. Validates formatting, linting, tests, documentation, and pop-cli specific patterns. |
| 4 | +--- |
| 5 | + |
| 6 | +# Final Review for Pop-CLI |
| 7 | + |
| 8 | +Comprehensive quality gate for pop-cli contributions ensuring code meets project standards. |
| 9 | + |
| 10 | +## When to Use |
| 11 | + |
| 12 | +- Before committing changes |
| 13 | +- Before creating a pull request |
| 14 | +- After completing a feature or fix |
| 15 | +- When user asks to "review", "check", "validate", or "finalize" changes |
| 16 | + |
| 17 | +## Quick Review Process |
| 18 | + |
| 19 | +### 1. Run Automated Checks |
| 20 | + |
| 21 | +Execute the validation script to check formatting, linting, and tests: |
| 22 | + |
| 23 | +```bash |
| 24 | +.claude/skills/final-review/scripts/validate.sh |
| 25 | +``` |
| 26 | + |
| 27 | +Or run checks individually: |
| 28 | + |
| 29 | +```bash |
| 30 | +# Formatting (nightly rustfmt, hard tabs) |
| 31 | +cargo +nightly fmt --all -- --check |
| 32 | + |
| 33 | +# Linting (deny all warnings) |
| 34 | +cargo clippy --all-targets --all-features -- -D warnings |
| 35 | + |
| 36 | +# Unit tests |
| 37 | +cargo test --workspace |
| 38 | + |
| 39 | +# Integration tests (if applicable) |
| 40 | +cargo test --features integration-tests |
| 41 | +``` |
| 42 | + |
| 43 | +### 2. Manual Review Checklist |
| 44 | + |
| 45 | +For each modified file, verify: |
| 46 | + |
| 47 | +**Code Quality:** |
| 48 | +- [ ] No `unwrap()` or `expect()` in library code (OK in tests/CLI entry) |
| 49 | +- [ ] Errors use `thiserror` with context via `anyhow::Context` |
| 50 | +- [ ] Functions returning results have descriptive error messages |
| 51 | +- [ ] No hardcoded paths or credentials |
| 52 | + |
| 53 | +**CLI Patterns (for `pop-cli` crate):** |
| 54 | +- [ ] Commands accept `&mut impl Cli` for testability |
| 55 | +- [ ] User-facing messages use `cliclack` methods (`intro`, `outro`, `success`, `warning`, `error`) |
| 56 | +- [ ] Long operations show progress indicators |
| 57 | +- [ ] `#[serde(skip_serializing)]` on sensitive fields (paths, secrets) |
| 58 | + |
| 59 | +**Feature Gates:** |
| 60 | +- [ ] New code properly feature-gated (`#[cfg(feature = "...")]`) |
| 61 | +- [ ] Features don't conflict in `Cargo.toml` |
| 62 | +- [ ] Default features include new functionality if broadly useful |
| 63 | + |
| 64 | +**Tests:** |
| 65 | +- [ ] Unit tests for new functions with complex logic |
| 66 | +- [ ] Integration tests for new commands |
| 67 | +- [ ] `MockCli` used for CLI interaction testing |
| 68 | +- [ ] No `#[ignore]` without documented reason |
| 69 | + |
| 70 | +### 3. Git Standards |
| 71 | + |
| 72 | +**Commit Message Format:** |
| 73 | +``` |
| 74 | +type(scope): short description |
| 75 | +
|
| 76 | +[optional body] |
| 77 | +
|
| 78 | +Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> |
| 79 | +``` |
| 80 | + |
| 81 | +**Types:** `feat`, `fix`, `refactor`, `docs`, `test`, `ci`, `chore` |
| 82 | +**Scopes:** `cli`, `contracts`, `chains`, `common`, `telemetry`, or specific command name |
| 83 | + |
| 84 | +### 4. Final Verification |
| 85 | + |
| 86 | +Before marking complete: |
| 87 | + |
| 88 | +```bash |
| 89 | +# Full CI simulation |
| 90 | +cargo +nightly fmt --all -- --check && \ |
| 91 | +cargo clippy --all-targets --all-features -- -D warnings && \ |
| 92 | +cargo test --workspace && \ |
| 93 | +cargo doc --no-deps |
| 94 | +``` |
| 95 | + |
| 96 | +## Detailed Reference |
| 97 | + |
| 98 | +For comprehensive checklists and edge cases, see [references/checklist.md](references/checklist.md). |
| 99 | + |
| 100 | +## Common Issues |
| 101 | + |
| 102 | +| Issue | Solution | |
| 103 | +|-------|----------| |
| 104 | +| Format fails | Run `cargo +nightly fmt --all` | |
| 105 | +| Clippy warnings | Fix or add `#[allow(...)]` with justification comment | |
| 106 | +| Test fails | Check recent changes, verify mocks are updated | |
| 107 | +| Feature conflict | Review `Cargo.toml` feature dependencies | |
| 108 | +| Missing docs | Add `///` doc comments to public items | |
0 commit comments