Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE.md
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,16 @@
See .github/checklists/contributor-self-review.template.md -->
- [ ] Self-review form completed and attached

## Contribution Quality Gate

<!-- Contributors: confirm your PR is ready for the maintainer quality gate.
Maintainers: run the checklist before approving.
See docs/contribution-quality-gate.md and
.github/checklists/contribution-quality-gate.md -->
- [ ] 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

<!-- Anything a reviewer should know: design decisions, trade-offs, follow-ups,
Expand All @@ -63,4 +73,6 @@
<!--
Reminder: a merged PR is NOT automatically payment-approved. Reward
eligibility is assessed separately (see the campaign's contribution terms).
Maintainers: do not approve until the Contribution Quality Gate checklist
passes (.github/checklists/contribution-quality-gate.md).
-->
89 changes: 89 additions & 0 deletions .github/checklists/contribution-quality-gate.md
Original file line number Diff line number Diff line change
@@ -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
23 changes: 0 additions & 23 deletions .github/pull_request_template.md

This file was deleted.

16 changes: 16 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
176 changes: 176 additions & 0 deletions docs/contribution-quality-gate.md
Original file line number Diff line number Diff line change
@@ -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
<!-- ❌ Issue: "add trustline preflight". PR only edits README. -->
```

**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.
4 changes: 4 additions & 0 deletions docs/meaningful-change-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
1 change: 1 addition & 0 deletions docs/pre-pr-verification.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`)