Skip to content

Commit 7efea9d

Browse files
committed
docs: add self-review checklist for mobile contributors (#448)
Add a pre-PR self-review checklist covering feature completion, tests, CI, screenshots/recordings, and device or emulator verification, and link it from README, CONTRIBUTING, and the PR template. Also fills the existing self-review-checklist.md link in the meaningful-change guide, which previously pointed nowhere. Closes #448
1 parent 69e3b5b commit 7efea9d

4 files changed

Lines changed: 125 additions & 3 deletions

File tree

.github/pull_request_template.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ runtime tests are not applicable and list the static review performed.
1414

1515
## Self-Assessment
1616

17-
Complete the
17+
Before opening this PR, run through the
18+
[Self-Review Checklist](https://github.com/Axionvera/pocketpay-mobile/blob/main/docs/self-review-checklist.md).
19+
Then complete the
1820
[Contributor Self-Assessment](https://github.com/Axionvera/pocketpay-mobile/blob/main/docs/contributor-self-assessment.md) before
1921
requesting review. Include concise evidence below; use
2022
`Not applicable — <reason>` instead of leaving a required area unexplained.

CONTRIBUTING.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,14 +214,16 @@ We strive to build a wallet that is accessible to everyone. Before submitting a
214214
git push -u origin feat/your-feature-name
215215
```
216216

217-
6. In the PR description:
217+
6. Before opening the PR, run through the [Self-Review Checklist](docs/self-review-checklist.md) — feature completion, tests, CI, screenshots, and device/emulator verification.
218+
219+
7. In the PR description:
218220
- Summarise what changed and why.
219221
- Reference any related issues using `Closes #<issue-number>`.
220222
- Describe how you tested the change.
221223
- Note any accessibility or security considerations.
222224
- For GrantFox mobile issues, review the [Evaluation Readiness Checklist](docs/evaluation-readiness-checklist.md) before the payment evaluation period. A merged PR does not guarantee payment approval.
223225

224-
7. A maintainer will review your PR. Please respond to feedback and update your branch as needed.
226+
8. A maintainer will review your PR. Please respond to feedback and update your branch as needed.
225227

226228
---
227229

README.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ React Native Expo wallet for Stellar Testnet. The app aims to feel polished and
1313

1414
- [Architecture Readiness Review](./docs/architecture-readiness-review.md) - Feature boundaries, duplicated state, SDK integration blockers, security-sensitive areas, and test gaps
1515
- [Evaluation Readiness Checklist](./docs/evaluation-readiness-checklist.md) - GrantFox contributor checklist for mobile issues, including tests, CI, screenshots, acceptance criteria, and the reminder that merge does not guarantee payment approval
16+
- [Self-Review Checklist](./docs/self-review-checklist.md) - Quick checklist to run before opening a PR, covering feature completion, tests, CI, screenshots, and device/emulator verification
1617
- [Contributor Self-Assessment](./docs/contributor-self-assessment.md) - Pre-review form for confirming scope, test evidence, CI, documentation, limitations, and acceptance criteria
1718
- [Storage Guide](./docs/storage.md) - SecureStore vs AsyncStorage
1819
- [Test-First Contribution Guide](./docs/test-first-contribution-guide.md) - Required test planning, happy-path and negative-path coverage, no-test justification rules, and local verification commands for mobile PRs
@@ -50,6 +51,7 @@ PocketPay Mobile is part of a broader PocketPay stack:
5051

5152
- [Screen Inventory](docs/screen-inventory.md) - A map of the main screens and routes in the app.
5253
- [Mobile Onboarding Checklist](docs/mobile-onboarding-checklist.md) - Quick-reference setup checklist for new contributors
54+
- [Self-Review Checklist](docs/self-review-checklist.md) - Pre-PR checklist covering feature completion, tests, CI, screenshots, and device/emulator verification
5355
- [Evaluation Readiness Checklist](docs/evaluation-readiness-checklist.md) - Mobile-issue review checklist for GrantFox contributors before payment evaluation
5456
- [UI State Catalogue](docs/ui-states.md) and [Accessibility Checklist](docs/accessibility.md) - Governance for major-screen states, shared component contracts, and accessible review evidence
5557
- [QR Receive Payload Format](docs/qr-payment-requests.md) - The address-only and SEP-0007-based payment-request formats the Receive screen encodes into its QR code

docs/self-review-checklist.md

Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
# Self-Review Checklist
2+
3+
## Purpose
4+
5+
A short, actionable checklist to run through **immediately before opening a
6+
PR** — not before payment evaluation. It exists so contributors can catch the
7+
same gaps a reviewer would catch, before a reviewer ever sees the diff.
8+
9+
This is not a replacement for the
10+
[Evaluation Readiness Checklist](evaluation-readiness-checklist.md) (the full
11+
pre-payment-evaluation bar) or the
12+
[Contributor Self-Assessment](contributor-self-assessment.md) (the form you
13+
paste into the PR description). Use this checklist first, as a fast pass over
14+
your own branch; use the other two afterward, when you're ready to write up
15+
evidence.
16+
17+
## When To Use This Checklist
18+
19+
- Right before you push your branch and click "Create Pull Request."
20+
- Again after any substantial change to an already-open PR.
21+
22+
## 1. Feature Completion
23+
24+
- [ ] Every item in the issue's Acceptance Criteria is implemented, or its
25+
absence is explicitly disclosed in the PR description.
26+
- [ ] The change matches the issue's Expected Behaviour, not a partial or
27+
adjacent version of it.
28+
- [ ] Every new interactive element is actually wired up — no button that
29+
renders but does nothing when pressed.
30+
- [ ] No unrelated files, renames, or formatting changes are bundled in.
31+
32+
## 2. Tests
33+
34+
- [ ] New or changed behaviour has a corresponding automated test, or the PR
35+
explains why a test isn't applicable.
36+
- [ ] Both the happy path and at least one negative/edge case are covered.
37+
- [ ] `npm test` passes locally on the latest commit.
38+
39+
## 3. CI
40+
41+
- [ ] `npm run typecheck` passes.
42+
- [ ] `npm run lint` passes.
43+
- [ ] `npm run api:check` passes if the change touches
44+
`src/types/pocketpay-sdk.d.ts` or `src/sdk-stub/` (see
45+
[SDK API Compatibility](sdk-api-compatibility.md)).
46+
- [ ] No required check is skipped, muted, or left failing on the latest
47+
commit. If a check is failing for a reason you can't explain, see the
48+
[CI Troubleshooting Guide](ci-troubleshooting.md) before opening the PR.
49+
50+
## 4. Screenshots Or Recordings
51+
52+
- [ ] A screenshot or short recording is attached for any change to visible
53+
UI or user flow.
54+
- [ ] Every state that changed is shown (e.g. loading, error, and success —
55+
not just the happy path).
56+
- [ ] The media shows the actual final result, not a mockup or an earlier
57+
draft of the change.
58+
- [ ] Screenshots contain only Testnet or dummy data — no real keys,
59+
balances, or personal data.
60+
- [ ] If the change is non-visual, the PR says so instead of leaving this
61+
section blank.
62+
63+
## 5. Device Or Emulator Verification
64+
65+
- [ ] The change was actually run on a physical device or a simulator/
66+
emulator — not verified by reading the code alone.
67+
- [ ] The app loads without a red error overlay or crash after the change.
68+
- [ ] The exact flow described in the issue was exercised end-to-end
69+
on-device, including any error or edge-case states the issue calls out.
70+
- [ ] If the change is platform-specific (iOS or Android only), it was
71+
verified on that platform rather than assumed to work from the other.
72+
73+
## 6. Documentation
74+
75+
- [ ] Any contributor, user, or architecture documentation affected by the
76+
change is updated, or the PR states why no update is needed.
77+
- [ ] README links, code comments, and examples touched by the change still
78+
match the current behaviour.
79+
80+
## Examples Of Incomplete Submissions
81+
82+
Each of these has shipped in a repo looking finished while failing one of the
83+
sections above:
84+
85+
- A "Copy Address" button that renders correctly in a screenshot but isn't
86+
wired to the clipboard — passes review 4 (screenshot) and fails review 1
87+
(feature completion).
88+
- A validation fix confirmed only by reading the diff, never launched in a
89+
simulator — fails review 5 even if the logic is correct.
90+
- A new screen with no entry in `__tests__/` or `tests/` — fails review 2
91+
regardless of how polished the UI looks.
92+
- A multi-state fix (e.g. locked / matured / withdrawn) that only handles one
93+
state, with the other two silently left broken — fails review 1 unless the
94+
gap is disclosed.
95+
- A PR description with an empty or missing Screenshots section on a change
96+
that clearly touches UI — fails review 4 by omission.
97+
98+
See the [Meaningful Change Guide](meaningful-change-guide.md) for the fuller
99+
set of examples and the reasoning behind them.
100+
101+
## Final Check
102+
103+
- [ ] I completed sections 1–6 above for this specific PR, not from memory of
104+
a previous one.
105+
- [ ] I would be comfortable if a reviewer checked every box above against my
106+
diff directly.
107+
108+
If any box is unchecked, treat the PR as not ready to open.
109+
110+
## Related Guidance
111+
112+
- [Contributor Self-Assessment](contributor-self-assessment.md)
113+
- [Evaluation Readiness Checklist](evaluation-readiness-checklist.md)
114+
- [Meaningful Change Guide](meaningful-change-guide.md)
115+
- [CI Troubleshooting Guide](ci-troubleshooting.md)
116+
- [Accessibility Checklist](accessibility.md)

0 commit comments

Comments
 (0)