diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index 11b65ad..f63e7d5 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -54,6 +54,16 @@ See .github/checklists/contributor-self-review.template.md --> - [ ] Self-review form completed and attached +## Contribution Quality Gate + + +- [ ] I reviewed the [Contribution Quality Gate](../docs/contribution-quality-gate.md) and believe this PR meets it +- [ ] Implementation is complete (not a stub / docs-only when behaviour was required) +- [ ] Tests, CI, docs, and acceptance criteria sections above are filled + ## Reviewer Notes diff --git a/.github/checklists/contribution-quality-gate.md b/.github/checklists/contribution-quality-gate.md new file mode 100644 index 0000000..e243352 --- /dev/null +++ b/.github/checklists/contribution-quality-gate.md @@ -0,0 +1,89 @@ +# Contribution Quality Gate Checklist + +> **Audience:** maintainers (and contributors preparing for review). +> Use this checklist **before approving** an SDK issue PR. +> Completing it does **not** approve GrantFox payment — that is assessed separately. + +**PR:** # +**Issue:** # +**Reviewer:** +**Date:** + +--- + +## 1. Meaningful implementation (size vs completeness) + +Size alone does not pass the gate. A small, complete fix can pass; a large +incomplete patch must not. + +- [ ] The PR solves the issue's stated problem (not a stub, comment-only, or symptom-only patch) +- [ ] Changes touch the necessary implementation modules (not docs-only when behaviour was required) +- [ ] Scope matches the issue — no unrelated drive-by refactors without rationale +- [ ] No leftover `TODO` / `FIXME` that block the acceptance criteria +- [ ] Public API changes (if any) are intentional and documented + +**Notes:** + +--- + +## 2. Tests + +- [ ] Behaviour changes include unit tests under `tests/` +- [ ] Failure / error paths are covered (not happy-path only) +- [ ] Bug fixes include a regression test +- [ ] Tests are offline-safe (no unmocked live Horizon / Friendbot calls) +- [ ] Docs-only / config-only PRs explicitly justify why tests are N/A + +**Test evidence (files or PR section):** + +--- + +## 3. CI status + +- [ ] Local gate was run (`npm run verify` and/or `npm run verify:pr`) +- [ ] All required GitHub CI checks on the PR are green +- [ ] Any red check is pre-existing, documented, and unrelated to this PR +- [ ] No secrets, `.env`, or credentials in the diff + +**CI notes / links:** + +--- + +## 4. Documentation + +- [ ] Public behaviour / API / error-code changes update `docs/` (or README) as needed +- [ ] Contributor-facing workflow changes update `CONTRIBUTING.md` when applicable +- [ ] Changelog / migration notes considered for user-facing changes +- [ ] Docs are accurate for the shipped behaviour (no aspirational-only text) + +**Docs touched:** + +--- + +## 5. Issue acceptance criteria + +- [ ] Every acceptance criterion from the issue is mapped in the PR description +- [ ] Each criterion is either satisfied or explicitly out-of-scope with rationale +- [ ] Contributor self-review form completed + (see [contributor-self-review.template.md](./contributor-self-review.template.md)) +- [ ] Pre-PR verification completed when applicable + (see [pre-pr-verification.md](../../docs/pre-pr-verification.md)) + +**Criteria gaps (if any):** + +--- + +## Gate decision + +- [ ] **PASS** — ready to approve / merge from a quality standpoint +- [ ] **HOLD** — missing items listed below; do not approve yet + +**Hold reasons / requested follow-ups:** + +--- + +## Related guides + +- [Contribution Quality Gate](../../docs/contribution-quality-gate.md) — full guidance and examples +- [Meaningful Change Review](../../docs/meaningful-change-review.md) — what counts as real SDK work +- [Contributor Self-Review Form](./contributor-self-review.template.md) — contributor-facing form diff --git a/.github/pull_request_template.md b/.github/pull_request_template.md deleted file mode 100644 index 31adf94..0000000 --- a/.github/pull_request_template.md +++ /dev/null @@ -1,23 +0,0 @@ -## Summary - - - -Closes # - -## Acceptance criteria - - - -- [ ] - -## Verification - -- [ ] I ran `npm run verify:pr` locally (see [Pre-PR Verification](../docs/pre-pr-verification.md)) -- [ ] If the issue has a checklist file, I ran `npm run verify:pr -- --checklist .github/checklists/issue-.md` -- [ ] All automated checks passed (lint, circular deps, unit tests, build) -- [ ] I confirmed each acceptance criterion above is satisfied -- [ ] I completed the [Contributor Self-Review Form](../.github/checklists/contributor-self-review.template.md) - -## Test plan - -- [ ] diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df0f14c..512aa2f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -200,6 +200,22 @@ The self-review covers: --- +## Contribution Quality Gate + +Maintainers use the [Contribution Quality Gate](./docs/contribution-quality-gate.md) before approving issue PRs. It is a repeatable checklist covering: + +- Meaningful implementation (completeness, not just patch size) +- Tests (including failure paths) +- CI status +- Documentation +- Issue acceptance criteria + +The checkbox form lives at [`.github/checklists/contribution-quality-gate.md`](.github/checklists/contribution-quality-gate.md). The guide includes examples of **incomplete** vs **acceptable** work. + +Contributors should read that guide before opening a PR. Maintainers should not approve until the checklist passes. A merged PR is still **not** automatic payment approval. + +--- + ## Reporting Issues Found a bug or have a feature idea? [Open an issue](https://github.com/Stellar-PocketPay/stellar-pocketpay-sdk/issues) and fill in as much detail as you can: diff --git a/README.md b/README.md index 8d50021..b047b3b 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,7 @@ npm install @axionvera/pocketpay-sdk - [SDK Roadmap](./docs/roadmap.md) - Directional plans and contributor opportunities across the SDK - [Testing](./docs/testing.md) - Unit vs integration test lanes and the offline guarantee - [Pre-PR Verification](./docs/pre-pr-verification.md) - Run `npm run verify:pr` before opening a pull request to confirm tests, docs, CI, and issue acceptance criteria +- [Contribution Quality Gate](./docs/contribution-quality-gate.md) - Maintainer checklist and examples of incomplete vs acceptable issue work before approval - [Getting Started](./docs/getting-started.md) - Step-by-step guide to install, create wallets, fund accounts, check balances, and send payments - [End-to-End App Integration Blueprint](./docs/app_integration_blueprint.md) - App-level flow combining config, diagnostics, wallet, account, payments, transactions, Soroban, vault, security, and typed error handling - [Testnet Account Funding](./docs/testnet-funding.md) - Funding and activating Testnet accounts with Friendbot, confirming activation, and common unfunded-account errors diff --git a/docs/contribution-quality-gate.md b/docs/contribution-quality-gate.md new file mode 100644 index 0000000..d5b57d7 --- /dev/null +++ b/docs/contribution-quality-gate.md @@ -0,0 +1,176 @@ +# Contribution Quality Gate + +A repeatable quality gate for PocketPay SDK issue work. Maintainers use it +**before approving** a PR. Contributors should treat it as the bar their PR +must clear. + +> **Merged ≠ payment-approved.** GrantFox and campaign rewards are assessed +> separately. This gate reduces incomplete merges that later fail evaluation. + +```text +Contributor self-review → npm run verify:pr → PR opened + ↓ +Maintainer quality gate checklist → approve / hold +``` + +## Checklist location + +Use the checklist in +[`.github/checklists/contribution-quality-gate.md`](../.github/checklists/contribution-quality-gate.md). + +It covers five areas required by this gate: + +| Area | What must be true | +| :--- | :--- | +| **Implementation** | Real behaviour in the right modules; size is not enough | +| **Tests** | Unit tests with failure paths; offline-safe | +| **CI status** | Local verify green; GitHub checks green (or documented pre-existing red) | +| **Docs** | Public contract / workflow docs updated when needed | +| **Acceptance criteria** | Every issue criterion mapped and satisfied (or scoped out) | + +Copy the checklist into a review comment or fill it while reviewing the PR. + +## How contributors prepare + +1. Complete the [Contributor Self-Review Form](../.github/checklists/contributor-self-review.template.md). +2. Run `npm run verify:pr` (see [Pre-PR Verification](./pre-pr-verification.md)). +3. Fill the PR template sections: tests, commands run, CI, acceptance criteria. +4. Map each issue acceptance criterion to a concrete change. + +## How maintainers validate + +Before clicking approve: + +1. Open [contribution-quality-gate.md](../.github/checklists/contribution-quality-gate.md). +2. Walk each section against the PR diff, CI tab, and issue acceptance criteria. +3. Cross-check with [Meaningful Change Review](./meaningful-change-review.md). +4. **PASS** only when every applicable box is checked. +5. **HOLD** with specific missing items if the gate fails — do not merge “to fix later” for incomplete criteria. + +### Quick maintainer questions + +- Does this PR solve the issue, or only touch nearby files / docs? +- Are failure paths tested, or only the happy path? +- Are GitHub checks green for *this* PR? +- Is every acceptance criterion accounted for in the description? +- Would this pass a later GrantFox evaluation as complete work? + +--- + +## Examples of incomplete work + +These patterns should **fail** the quality gate. + +### 1. Tiny stub for a behaviour issue + +```ts +// ❌ Issue asked for validation; PR returns input unchanged. +export function validateSendXLMParams(params: SendXLMParams) { + return { ok: true as const }; +} +``` + +**Why it fails:** no real implementation; acceptance criteria unmet. + +### 2. Implementation without tests + +```ts +// ❌ New classify path shipped with zero tests. +export function classifySubmitError(error: unknown): PocketPayError { /* ... */ } +``` + +**Why it fails:** tests section of the gate is empty; regression risk. + +### 3. Happy-path-only tests + +```ts +it('sends payment', async () => { + await expect(sendXLM(valid)).resolves.toMatchObject({ success: true }); +}); +// ❌ Never asserts INVALID_AMOUNT, ACCOUNT_NOT_FOUND, PAYMENT_FAILED, etc. +``` + +**Why it fails:** failure-path coverage required for payment / error work. + +### 4. Docs-only PR for a code issue + +```md + +``` + +**Why it fails:** implementation size/completeness gate — behaviour was required. + +### 5. Greenwashed CI + +```text +Local: tests fail +PR body: "CI will fix it" +CI: red, unexplained +``` + +**Why it fails:** CI status gate — checks must be green or pre-existing red must be documented. + +### 6. Acceptance criteria ignored + +```md +## Acceptance criteria +- [ ] (left blank) +``` + +**Why it fails:** criteria mapping is mandatory; blank sections fail the gate. + +--- + +## Examples of acceptable work + +These patterns should **pass** the quality gate when CI is green. + +### 1. Complete behaviour + tests + criteria + +- Implements the rule in the owning module (e.g. `src/payments/`). +- Adds `tests/*.test.ts` covering success **and** typed error paths. +- `npm run verify` / `npm run verify:pr` green; GitHub checks green. +- PR description checks every issue acceptance criterion. +- Docs updated if the public contract changed. + +### 2. Focused bugfix with regression test + +- Minimal diff that fixes the root cause. +- New failing test that would have caught the bug, then made green. +- No unrelated refactors. +- Criterion “regression test added” explicitly checked. + +### 3. Docs / DX workflow issue done end-to-end + +- When the issue is documentation or contributor tooling (e.g. this quality gate): + - checklist / guide files exist, + - PR template and README link them, + - examples of incomplete vs acceptable work are present, + - no fake `src/` stubs required. +- Still runs `npm run verify` if package scripts or tests were touched. + +### 4. Refactor that preserves behaviour + +- Existing tests still pass; new coverage where gaps were found. +- No silent public API break. +- Rationale documented in the PR. + +--- + +## Relationship to other docs + +| Doc | Role | +| :--- | :--- | +| [contribution-quality-gate.md](../.github/checklists/contribution-quality-gate.md) | Maintainer checkbox form | +| [Contributor Self-Review](../.github/checklists/contributor-self-review.template.md) | Contributor form before review | +| [Meaningful Change Review](./meaningful-change-review.md) | What “meaningful” SDK work looks like | +| [Pre-PR Verification](./pre-pr-verification.md) | `npm run verify:pr` automated reminders | +| [Local Verification](./local-verification.md) | `npm run verify` pipeline details | + +--- + +## PR template + +The [PR template](../.github/PULL_REQUEST_TEMPLATE.md) asks contributors to +acknowledge this quality gate. Maintainers should still run the full checklist +before approval — the template alone is not a pass. diff --git a/docs/meaningful-change-review.md b/docs/meaningful-change-review.md index e6c932f..cb516b4 100644 --- a/docs/meaningful-change-review.md +++ b/docs/meaningful-change-review.md @@ -136,3 +136,7 @@ public contract changed. If the answer to the first question is "just code/comments," the PR is not meaningful yet — request the missing behaviour, tests, or criteria coverage before approving. Merging it does not by itself approve payment. + +For the full maintainer **pass / hold** checklist (implementation, tests, CI, +docs, acceptance criteria) and more incomplete vs acceptable examples, see +[Contribution Quality Gate](./contribution-quality-gate.md). diff --git a/docs/pre-pr-verification.md b/docs/pre-pr-verification.md index be32a1a..604ec5f 100644 --- a/docs/pre-pr-verification.md +++ b/docs/pre-pr-verification.md @@ -130,5 +130,6 @@ PocketPay SDK — Pre-PR Acceptance Verification ## Related docs - [CONTRIBUTING.md](../CONTRIBUTING.md) — contributor setup and PR checklist +- [Contribution Quality Gate](./contribution-quality-gate.md) — maintainer pass/hold checklist before approval - [testing.md](./testing.md) — unit vs integration test lanes - [release-checklist.md](./release-checklist.md) — maintainer release gates (`npm run verify`)