From 826a0b4143547d19f05def80297e090700689bc8 Mon Sep 17 00:00:00 2001 From: CleanDev-Fix <219162456+CleanDev-Fix@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:05:55 -0400 Subject: [PATCH] docs: add meaningful change threshold guide --- README.md | 3 ++ docs/contribution-quality-gate.md | 5 ++ docs/meaningful-change-threshold.md | 80 +++++++++++++++++++++++++++++ 3 files changed, 88 insertions(+) create mode 100644 docs/meaningful-change-threshold.md diff --git a/README.md b/README.md index f911e64..990bba6 100644 --- a/README.md +++ b/README.md @@ -198,6 +198,8 @@ stellar-pocketpay-contracts/ ├── Cargo.toml # Workspace root ├── .gitignore ├── README.md +├── docs/ +│ └── meaningful-change-threshold.md # Contribution scope review guide └── contracts/ └── savings_vault/ ├── Cargo.toml # Contract crate @@ -213,6 +215,7 @@ stellar-pocketpay-contracts/ - [Admin Role](docs/admin-role.md) – Details on the admin address, current capabilities, and future design considerations. - [Admin & Emergency Mechanism Threat Model](docs/admin-pause-threat-model.md) – Security analysis of malicious admin, compromised admin, accidental pause, and blocked-withdrawal scenarios. - [Failure Mode Catalogue](docs/failure-mode-catalogue.md) – Summary of safe-failure behavior, expected errors, affected functions, and related tests for vault operations. +- [Meaningful Change Threshold](docs/meaningful-change-threshold.md) – Outcome- and evidence-based guidance for distinguishing small complete changes from small incomplete changes. - [Contributor Self-Review Template](docs/self-review-template.md) – Copy-paste checklist covering behaviour, tests, CI, security, edge cases, and docs impact — fill it in before opening a PR. - [Traceability Table Guide](docs/traceability-table.md) – Standard format for mapping PR changes to issue acceptance criteria, with worked examples. - [Payment-Period Conduct Guidance](docs/payment-period-conduct.md) – Expectations for how contributors raise payment-status questions, and how GrantFox's evaluation process relates to this repository's review process. diff --git a/docs/contribution-quality-gate.md b/docs/contribution-quality-gate.md index 2270577..d151685 100644 --- a/docs/contribution-quality-gate.md +++ b/docs/contribution-quality-gate.md @@ -2,6 +2,11 @@ This document defines the objective criteria for "payment-ready" or "production-ready" work in the PocketPay Contracts repository. To maintain high security and reliability, every Pull Request (PR) must pass through this quality gate before being considered for final approval. +Change size is not a quality requirement. Use the +[Meaningful Change Threshold](meaningful-change-threshold.md) to distinguish a +small complete change from a small incomplete one and to assess scope without +using line count as a proxy for value. + ## 1. Quality Gate Checklist Every PR must satisfy the following checklist. If any item is missing or incomplete, the PR will be flagged for further work. diff --git a/docs/meaningful-change-threshold.md b/docs/meaningful-change-threshold.md new file mode 100644 index 0000000..9adb24f --- /dev/null +++ b/docs/meaningful-change-threshold.md @@ -0,0 +1,80 @@ +# Meaningful Change Threshold + +PocketPay Contracts evaluates a contribution by the complete outcome it +delivers, not by a minimum number of changed lines. A short diff can be the +right fix, while a large diff can still leave the issue incomplete. + +This guide complements the [Contribution Quality Gate](contribution-quality-gate.md). +The quality gate remains the source of truth for contract safety, tests, +documentation, and local verification requirements. + +## What makes a change meaningful + +A change is meaningful when the pull request provides enough evidence for a +reviewer to conclude that it: + +1. addresses the linked issue's complete agreed scope; +2. implements the intended behavior without placeholders or unrelated work; +3. includes the tests and edge cases appropriate to the risk; +4. updates documentation and traceability where the public contract or + contributor workflow changes; and +5. passes the repository's relevant verification and CI checks. + +Line count can help a reviewer notice an unexpectedly small or broad diff, but +it is only a prompt to inspect the evidence. It is not an acceptance criterion +and must not be used as a substitute for technical review. + +## Small complete changes and small incomplete changes + +| Change | Small but complete | Small and incomplete | +|---|---|---| +| Boundary fix | Changes the exact comparison, adds tests immediately below, at, and above the boundary, and maps the result to the issue criteria. | Changes the comparison but adds no boundary or regression test. | +| Error mapping | Adds the missing `ContractError` mapping, covers the failure path, and updates error documentation when public behavior changes. | Adds a new error variant without wiring every throw path or documenting the public result. | +| Documentation correction | Fixes the incorrect guidance at its source, updates affected links, and verifies that the links resolve. | Adds a second explanation elsewhere while leaving the incorrect source unchanged. | +| Focused refactor | Removes duplication without changing behavior and demonstrates that the existing relevant tests still pass. | Moves code while leaving dead paths, placeholders, or unexplained behavior differences. | + +A small complete change should not be padded with unrelated files or cosmetic +edits. An incomplete change should not be accepted merely because it has many +lines. + +## Insufficient examples + +The following do not meet the meaningful-change threshold: + +- a comment, rename, or formatting-only diff presented as a behavioral fix; +- a happy-path implementation when the issue requires authorization, failure, + or boundary behavior; +- a new function with `TODO`, `FIXME`, mocked return values, or unreachable + integration points left in the requested scope; +- tests that only restate implementation details and do not prove the issue's + observable behavior; +- documentation that duplicates existing guidance instead of updating or + linking the repository's source of truth; +- a broad cleanup that obscures the issue-specific change and makes its + evidence difficult to review; or +- a pull request that omits acceptance criteria, known limitations, or relevant + verification results. + +## Reviewer assessment + +Reviewers should use this sequence rather than counting lines: + +1. **Confirm scope.** Compare the diff with the linked issue and its acceptance + criteria. Identify any requested behavior that is absent. +2. **Trace the outcome.** Follow each criterion to implementation, + documentation, and test evidence in the pull request's + [traceability table](traceability-table.md). +3. **Check risk coverage.** Apply the contract-specific checks in the + [Contribution Quality Gate](contribution-quality-gate.md), including + authorization, rollback, invariant, and edge-case expectations when they + are relevant. +4. **Check integration.** Confirm the change is wired into the actual contract + or contributor workflow rather than added as an unused helper or isolated + document. +5. **Classify the result.** Approve a small change when the evidence is complete; + request the missing issue-scoped work when it is not. Ask for unrelated + improvements in a follow-up issue rather than padding the current pull + request. + +If the issue itself is ambiguous, reviewers should clarify the intended scope +before using this guide to judge completeness.