Skip to content

Commit 22c5761

Browse files
panditdhamdherepanditdhamdhere
committed
Add contribution quality gate for issue completion (#366)
Add a maintainer quality-gate checklist and guide covering implementation, tests, CI, docs, and acceptance criteria, with incomplete vs acceptable examples. Wire the PR template, README, and CONTRIBUTING to the gate.
1 parent c6f94d4 commit 22c5761

8 files changed

Lines changed: 299 additions & 23 deletions

‎.github/PULL_REQUEST_TEMPLATE.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,16 @@
5454
See .github/checklists/contributor-self-review.template.md -->
5555
- [ ] Self-review form completed and attached
5656

57+
## Contribution Quality Gate
58+
59+
<!-- Contributors: confirm your PR is ready for the maintainer quality gate.
60+
Maintainers: run the checklist before approving.
61+
See docs/contribution-quality-gate.md and
62+
.github/checklists/contribution-quality-gate.md -->
63+
- [ ] I reviewed the [Contribution Quality Gate](../docs/contribution-quality-gate.md) and believe this PR meets it
64+
- [ ] Implementation is complete (not a stub / docs-only when behaviour was required)
65+
- [ ] Tests, CI, docs, and acceptance criteria sections above are filled
66+
5767
## Reviewer Notes
5868

5969
<!-- Anything a reviewer should know: design decisions, trade-offs, follow-ups,
@@ -63,4 +73,6 @@
6373
<!--
6474
Reminder: a merged PR is NOT automatically payment-approved. Reward
6575
eligibility is assessed separately (see the campaign's contribution terms).
76+
Maintainers: do not approve until the Contribution Quality Gate checklist
77+
passes (.github/checklists/contribution-quality-gate.md).
6678
-->

‎.github/checklists/contribution-quality-gate.md‎

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
# Contribution Quality Gate Checklist
2+
3+
> **Audience:** maintainers (and contributors preparing for review).
4+
> Use this checklist **before approving** an SDK issue PR.
5+
> Completing it does **not** approve GrantFox payment — that is assessed separately.
6+
7+
**PR:** #
8+
**Issue:** #
9+
**Reviewer:**
10+
**Date:**
11+
12+
---
13+
14+
## 1. Meaningful implementation (size vs completeness)
15+
16+
Size alone does not pass the gate. A small, complete fix can pass; a large
17+
incomplete patch must not.
18+
19+
- [ ] The PR solves the issue's stated problem (not a stub, comment-only, or symptom-only patch)
20+
- [ ] Changes touch the necessary implementation modules (not docs-only when behaviour was required)
21+
- [ ] Scope matches the issue — no unrelated drive-by refactors without rationale
22+
- [ ] No leftover `TODO` / `FIXME` that block the acceptance criteria
23+
- [ ] Public API changes (if any) are intentional and documented
24+
25+
**Notes:**
26+
27+
---
28+
29+
## 2. Tests
30+
31+
- [ ] Behaviour changes include unit tests under `tests/`
32+
- [ ] Failure / error paths are covered (not happy-path only)
33+
- [ ] Bug fixes include a regression test
34+
- [ ] Tests are offline-safe (no unmocked live Horizon / Friendbot calls)
35+
- [ ] Docs-only / config-only PRs explicitly justify why tests are N/A
36+
37+
**Test evidence (files or PR section):**
38+
39+
---
40+
41+
## 3. CI status
42+
43+
- [ ] Local gate was run (`npm run verify` and/or `npm run verify:pr`)
44+
- [ ] All required GitHub CI checks on the PR are green
45+
- [ ] Any red check is pre-existing, documented, and unrelated to this PR
46+
- [ ] No secrets, `.env`, or credentials in the diff
47+
48+
**CI notes / links:**
49+
50+
---
51+
52+
## 4. Documentation
53+
54+
- [ ] Public behaviour / API / error-code changes update `docs/` (or README) as needed
55+
- [ ] Contributor-facing workflow changes update `CONTRIBUTING.md` when applicable
56+
- [ ] Changelog / migration notes considered for user-facing changes
57+
- [ ] Docs are accurate for the shipped behaviour (no aspirational-only text)
58+
59+
**Docs touched:**
60+
61+
---
62+
63+
## 5. Issue acceptance criteria
64+
65+
- [ ] Every acceptance criterion from the issue is mapped in the PR description
66+
- [ ] Each criterion is either satisfied or explicitly out-of-scope with rationale
67+
- [ ] Contributor self-review form completed
68+
(see [contributor-self-review.template.md](./contributor-self-review.template.md))
69+
- [ ] Pre-PR verification completed when applicable
70+
(see [pre-pr-verification.md](../../docs/pre-pr-verification.md))
71+
72+
**Criteria gaps (if any):**
73+
74+
---
75+
76+
## Gate decision
77+
78+
- [ ] **PASS** — ready to approve / merge from a quality standpoint
79+
- [ ] **HOLD** — missing items listed below; do not approve yet
80+
81+
**Hold reasons / requested follow-ups:**
82+
83+
---
84+
85+
## Related guides
86+
87+
- [Contribution Quality Gate](../../docs/contribution-quality-gate.md) — full guidance and examples
88+
- [Meaningful Change Review](../../docs/meaningful-change-review.md) — what counts as real SDK work
89+
- [Contributor Self-Review Form](./contributor-self-review.template.md) — contributor-facing form

‎.github/pull_request_template.md‎

Lines changed: 0 additions & 23 deletions
This file was deleted.

‎CONTRIBUTING.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,22 @@ The self-review covers:
200200
201201
---
202202

203+
## Contribution Quality Gate
204+
205+
Maintainers use the [Contribution Quality Gate](./docs/contribution-quality-gate.md) before approving issue PRs. It is a repeatable checklist covering:
206+
207+
- Meaningful implementation (completeness, not just patch size)
208+
- Tests (including failure paths)
209+
- CI status
210+
- Documentation
211+
- Issue acceptance criteria
212+
213+
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.
214+
215+
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.
216+
217+
---
218+
203219
## Reporting Issues
204220

205221
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:

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ npm install @axionvera/pocketpay-sdk
5151
- [SDK Roadmap](./docs/roadmap.md) - Directional plans and contributor opportunities across the SDK
5252
- [Testing](./docs/testing.md) - Unit vs integration test lanes and the offline guarantee
5353
- [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
54+
- [Contribution Quality Gate](./docs/contribution-quality-gate.md) - Maintainer checklist and examples of incomplete vs acceptable issue work before approval
5455
- [Getting Started](./docs/getting-started.md) - Step-by-step guide to install, create wallets, fund accounts, check balances, and send payments
5556
- [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
5657
- [Testnet Account Funding](./docs/testnet-funding.md) - Funding and activating Testnet accounts with Friendbot, confirming activation, and common unfunded-account errors

‎docs/contribution-quality-gate.md‎

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
# Contribution Quality Gate
2+
3+
A repeatable quality gate for PocketPay SDK issue work. Maintainers use it
4+
**before approving** a PR. Contributors should treat it as the bar their PR
5+
must clear.
6+
7+
> **Merged ≠ payment-approved.** GrantFox and campaign rewards are assessed
8+
> separately. This gate reduces incomplete merges that later fail evaluation.
9+
10+
```text
11+
Contributor self-review → npm run verify:pr → PR opened
12+
↓
13+
Maintainer quality gate checklist → approve / hold
14+
```
15+
16+
## Checklist location
17+
18+
Use the checklist in
19+
[`.github/checklists/contribution-quality-gate.md`](../.github/checklists/contribution-quality-gate.md).
20+
21+
It covers five areas required by this gate:
22+
23+
| Area | What must be true |
24+
| :--- | :--- |
25+
| **Implementation** | Real behaviour in the right modules; size is not enough |
26+
| **Tests** | Unit tests with failure paths; offline-safe |
27+
| **CI status** | Local verify green; GitHub checks green (or documented pre-existing red) |
28+
| **Docs** | Public contract / workflow docs updated when needed |
29+
| **Acceptance criteria** | Every issue criterion mapped and satisfied (or scoped out) |
30+
31+
Copy the checklist into a review comment or fill it while reviewing the PR.
32+
33+
## How contributors prepare
34+
35+
1. Complete the [Contributor Self-Review Form](../.github/checklists/contributor-self-review.template.md).
36+
2. Run `npm run verify:pr` (see [Pre-PR Verification](./pre-pr-verification.md)).
37+
3. Fill the PR template sections: tests, commands run, CI, acceptance criteria.
38+
4. Map each issue acceptance criterion to a concrete change.
39+
40+
## How maintainers validate
41+
42+
Before clicking approve:
43+
44+
1. Open [contribution-quality-gate.md](../.github/checklists/contribution-quality-gate.md).
45+
2. Walk each section against the PR diff, CI tab, and issue acceptance criteria.
46+
3. Cross-check with [Meaningful Change Review](./meaningful-change-review.md).
47+
4. **PASS** only when every applicable box is checked.
48+
5. **HOLD** with specific missing items if the gate fails — do not merge “to fix later” for incomplete criteria.
49+
50+
### Quick maintainer questions
51+
52+
- Does this PR solve the issue, or only touch nearby files / docs?
53+
- Are failure paths tested, or only the happy path?
54+
- Are GitHub checks green for *this* PR?
55+
- Is every acceptance criterion accounted for in the description?
56+
- Would this pass a later GrantFox evaluation as complete work?
57+
58+
---
59+
60+
## Examples of incomplete work
61+
62+
These patterns should **fail** the quality gate.
63+
64+
### 1. Tiny stub for a behaviour issue
65+
66+
```ts
67+
// ❌ Issue asked for validation; PR returns input unchanged.
68+
export function validateSendXLMParams(params: SendXLMParams) {
69+
return { ok: true as const };
70+
}
71+
```
72+
73+
**Why it fails:** no real implementation; acceptance criteria unmet.
74+
75+
### 2. Implementation without tests
76+
77+
```ts
78+
// ❌ New classify path shipped with zero tests.
79+
export function classifySubmitError(error: unknown): PocketPayError { /* ... */ }
80+
```
81+
82+
**Why it fails:** tests section of the gate is empty; regression risk.
83+
84+
### 3. Happy-path-only tests
85+
86+
```ts
87+
it('sends payment', async () => {
88+
await expect(sendXLM(valid)).resolves.toMatchObject({ success: true });
89+
});
90+
// ❌ Never asserts INVALID_AMOUNT, ACCOUNT_NOT_FOUND, PAYMENT_FAILED, etc.
91+
```
92+
93+
**Why it fails:** failure-path coverage required for payment / error work.
94+
95+
### 4. Docs-only PR for a code issue
96+
97+
```md
98+
<!-- ❌ Issue: "add trustline preflight". PR only edits README. -->
99+
```
100+
101+
**Why it fails:** implementation size/completeness gate — behaviour was required.
102+
103+
### 5. Greenwashed CI
104+
105+
```text
106+
Local: tests fail
107+
PR body: "CI will fix it"
108+
CI: red, unexplained
109+
```
110+
111+
**Why it fails:** CI status gate — checks must be green or pre-existing red must be documented.
112+
113+
### 6. Acceptance criteria ignored
114+
115+
```md
116+
## Acceptance criteria
117+
- [ ] (left blank)
118+
```
119+
120+
**Why it fails:** criteria mapping is mandatory; blank sections fail the gate.
121+
122+
---
123+
124+
## Examples of acceptable work
125+
126+
These patterns should **pass** the quality gate when CI is green.
127+
128+
### 1. Complete behaviour + tests + criteria
129+
130+
- Implements the rule in the owning module (e.g. `src/payments/`).
131+
- Adds `tests/*.test.ts` covering success **and** typed error paths.
132+
- `npm run verify` / `npm run verify:pr` green; GitHub checks green.
133+
- PR description checks every issue acceptance criterion.
134+
- Docs updated if the public contract changed.
135+
136+
### 2. Focused bugfix with regression test
137+
138+
- Minimal diff that fixes the root cause.
139+
- New failing test that would have caught the bug, then made green.
140+
- No unrelated refactors.
141+
- Criterion “regression test added” explicitly checked.
142+
143+
### 3. Docs / DX workflow issue done end-to-end
144+
145+
- When the issue is documentation or contributor tooling (e.g. this quality gate):
146+
- checklist / guide files exist,
147+
- PR template and README link them,
148+
- examples of incomplete vs acceptable work are present,
149+
- no fake `src/` stubs required.
150+
- Still runs `npm run verify` if package scripts or tests were touched.
151+
152+
### 4. Refactor that preserves behaviour
153+
154+
- Existing tests still pass; new coverage where gaps were found.
155+
- No silent public API break.
156+
- Rationale documented in the PR.
157+
158+
---
159+
160+
## Relationship to other docs
161+
162+
| Doc | Role |
163+
| :--- | :--- |
164+
| [contribution-quality-gate.md](../.github/checklists/contribution-quality-gate.md) | Maintainer checkbox form |
165+
| [Contributor Self-Review](../.github/checklists/contributor-self-review.template.md) | Contributor form before review |
166+
| [Meaningful Change Review](./meaningful-change-review.md) | What “meaningful” SDK work looks like |
167+
| [Pre-PR Verification](./pre-pr-verification.md) | `npm run verify:pr` automated reminders |
168+
| [Local Verification](./local-verification.md) | `npm run verify` pipeline details |
169+
170+
---
171+
172+
## PR template
173+
174+
The [PR template](../.github/PULL_REQUEST_TEMPLATE.md) asks contributors to
175+
acknowledge this quality gate. Maintainers should still run the full checklist
176+
before approval — the template alone is not a pass.

‎docs/meaningful-change-review.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,3 +136,7 @@ public contract changed.
136136
If the answer to the first question is "just code/comments," the PR is not
137137
meaningful yet — request the missing behaviour, tests, or criteria coverage
138138
before approving. Merging it does not by itself approve payment.
139+
140+
For the full maintainer **pass / hold** checklist (implementation, tests, CI,
141+
docs, acceptance criteria) and more incomplete vs acceptable examples, see
142+
[Contribution Quality Gate](./contribution-quality-gate.md).

‎docs/pre-pr-verification.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,5 +130,6 @@ PocketPay SDK — Pre-PR Acceptance Verification
130130
## Related docs
131131

132132
- [CONTRIBUTING.md](../CONTRIBUTING.md) — contributor setup and PR checklist
133+
- [Contribution Quality Gate](./contribution-quality-gate.md) — maintainer pass/hold checklist before approval
133134
- [testing.md](./testing.md) — unit vs integration test lanes
134135
- [release-checklist.md](./release-checklist.md) — maintainer release gates (`npm run verify`)

0 commit comments

Comments
 (0)