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
4 changes: 3 additions & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ runtime tests are not applicable and list the static review performed.

## Self-Assessment

Complete the
Before opening this PR, run through the
[Self-Review Checklist](https://github.com/Axionvera/pocketpay-mobile/blob/main/docs/self-review-checklist.md).
Then complete the
[Contributor Self-Assessment](https://github.com/Axionvera/pocketpay-mobile/blob/main/docs/contributor-self-assessment.md) before
requesting review. Include concise evidence below; use
`Not applicable — <reason>` instead of leaving a required area unexplained.
Expand Down
6 changes: 4 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,14 +214,16 @@ We strive to build a wallet that is accessible to everyone. Before submitting a
git push -u origin feat/your-feature-name
```

6. In the PR description:
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.

7. In the PR description:
- Summarise what changed and why.
- Reference any related issues using `Closes #<issue-number>`.
- Describe how you tested the change.
- Note any accessibility or security considerations.
- 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.

7. A maintainer will review your PR. Please respond to feedback and update your branch as needed.
8. A maintainer will review your PR. Please respond to feedback and update your branch as needed.

---

Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ React Native Expo wallet for Stellar Testnet. The app aims to feel polished and

- [Architecture Readiness Review](./docs/architecture-readiness-review.md) - Feature boundaries, duplicated state, SDK integration blockers, security-sensitive areas, and test gaps
- [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
- [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
- [Contributor Self-Assessment](./docs/contributor-self-assessment.md) - Pre-review form for confirming scope, test evidence, CI, documentation, limitations, and acceptance criteria
- [Storage Guide](./docs/storage.md) - SecureStore vs AsyncStorage
- [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
Expand Down Expand Up @@ -50,6 +51,7 @@ PocketPay Mobile is part of a broader PocketPay stack:

- [Screen Inventory](docs/screen-inventory.md) - A map of the main screens and routes in the app.
- [Mobile Onboarding Checklist](docs/mobile-onboarding-checklist.md) - Quick-reference setup checklist for new contributors
- [Self-Review Checklist](docs/self-review-checklist.md) - Pre-PR checklist covering feature completion, tests, CI, screenshots, and device/emulator verification
- [Evaluation Readiness Checklist](docs/evaluation-readiness-checklist.md) - Mobile-issue review checklist for GrantFox contributors before payment evaluation
- [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
- [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
Expand Down
135 changes: 135 additions & 0 deletions docs/meaningful-change-guide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
# What Counts as Meaningful Work

A PR that technically merges is not the same as a PR that actually solves an
issue. This guide exists because small mobile PRs can change styling, rename
a variable, or touch a few unrelated lines without addressing the behaviour
an issue actually asked for — and merging does not, by itself, mean the work
is complete or payment-ready.

> ⚠️ **Merging a PR does not guarantee payment approval.** GrantFox
> evaluation looks at whether the requested behaviour, tests, and acceptance
> criteria were actually delivered — not just whether the diff was accepted.
> See the [Evaluation Readiness Checklist](evaluation-readiness-checklist.md)
> for the full bar, and the
> [Self-Review Checklist](self-review-checklist.md) to check your own work
> against it before you open a PR.

## The Bar: What "Meaningful" Requires

A change is meaningful when it addresses **all** of the following for the
issue it claims to close — not just the parts that were easy:

1. **The requested behaviour**, as written in the issue's Expected Behaviour
and Acceptance Criteria — not a partial or adjacent version of it.
2. **Tests** for the new or changed behaviour. Check the screen's row in the
[Screen Test Matrix](screen-test-matrix.md) — if the screen you touched
has a ❌ or ⚠️ there, "I didn't add a test" is now a visible gap, not an
invisible one.
3. **Screenshots or a screen recording** for any visual change. A reviewer
should not have to pull your branch and run the app just to see what you
built.
4. **Accessibility**, where the change touches UI — see the
[Accessibility Checklist](accessibility.md). A new interactive element
without an `accessibilityLabel` is not a finished element.
5. **Every item in the issue's acceptance criteria**, checked individually
against your actual diff. Partially solving 3 of 6 checkboxes and opening
the PR anyway is a different (and acceptable, if disclosed) thing from
silently claiming the issue is done.

## Examples of Incomplete Changes

These are realistic shapes an incomplete PR takes in this codebase — the
common thread is that each one *looks* like it addresses the issue title
without actually delivering the behaviour, tests, or evidence the issue
asked for.

- **Styling-only fix for a functional bug.** The issue asks for the Send
screen to show a validation error when the amount exceeds the balance; the
PR only restyles the existing (unrelated) error banner and adds no
validation logic or test. The title matches the issue; the diff doesn't.
- **Dead UI.** A new "Copy Address" button is added to the Receive screen,
but it isn't wired to the clipboard — pressing it does nothing. It looks
finished in a screenshot and is not finished at all.
- **Happy-path-only fix for a multi-state issue.** An issue about the Vault
lock list asks for correct behaviour across locked, matured, and withdrawn
states; the PR only handles the locked case and the other two are
silently left broken, with no test and no note that they're out of scope.
- **No test for new behaviour.** A new screen or interactive control is
added with zero corresponding entry in `__tests__/` or `tests/`, and the
[Screen Test Matrix](screen-test-matrix.md) is not updated to reflect the
new screen at all — the next contributor has no way to know the gap
exists.
- **No visual evidence for a visual change.** A PR changes a screen's layout
or adds a new state (loading/empty/error) but includes no screenshot or
recording, so the reviewer either has to trust the description or spend
their own time reproducing it.
- **Accessibility left out.** A new icon-only button (e.g. a delete or copy
action) ships with no `accessible`/`accessibilityLabel`, failing the
[Accessibility Checklist](accessibility.md) minimums silently, because
nothing in CI catches missing accessibility props.
- **Unrelated scope bundled in.** A PR "fixes" the requested issue but also
reformats unrelated files or renames unrelated variables, making the diff
harder to review and harder to evaluate against the issue alone.

## Examples of Acceptable Changes

- **A full validation fix.** The Send-screen validation issue above, done
completely: the validation logic is added, the error state matches the
[UI State Catalogue](ui-states.md#send) Error row, a test exercises the
new validation branch (both the failing and passing case), and the PR
description includes a screenshot of the new error state.
- **A new screen, fully landed.** A new screen ships with: a component test,
a new row added to the [Screen Test Matrix](screen-test-matrix.md),
accessible labels on every interactive element, and a screenshot or
recording in the PR description showing it rendered on-device or in a
simulator.
- **An honestly partial PR.** The PR covers wallet creation but not wallet
import from the same issue, and the description says so explicitly —
"Import is intentionally out of scope here; tracked as a follow-up because
X" — instead of implying full coverage. Disclosed partial scope is a
legitimate outcome; undisclosed partial scope is not.
- **A bug fix with a regression test.** The fix includes a test that would
have failed before the fix and passes after it, so the specific bug can't
silently come back.

## Required Testing Evidence

Every PR should let a reviewer verify the work **without** re-running it
themselves:

- The exact commands you ran and their result: `npm run typecheck`,
`npm run lint`, `npm test` (see the
[CI Troubleshooting Guide](ci-troubleshooting.md) if any of them fail for
a reason you can't explain — don't open a PR with a known-failing check).
- For any UI change: a screenshot or short recording of the changed
screen/state. For a multi-state change (e.g. adding an error state),
show each state that changed, not just the happy path.
- For a behavioural fix: which acceptance-criteria checkbox(es) from the
issue each part of your change satisfies. Don't make the reviewer guess
which line addresses which requirement.

## Reviewer Expectations

- Reviewers check the PR against the issue's acceptance criteria **line by
line**, not against the PR title or description alone.
- If a claimed behaviour has no test and no screenshot, a reviewer may ask
for one before approving — that's expected, not a sign something went
wrong.
- A green CI run is necessary but not sufficient. CI catches typechecking,
lint, and test failures; it does not catch a validation rule that was
never implemented, or an accessibility label that was never added.
- Scope that's intentionally left out should be stated in the PR
description. Scope that's silently missing is treated as incomplete work,
not as an acceptable interpretation of the issue.

## See Also

- [Evaluation Readiness Checklist](evaluation-readiness-checklist.md) — the
full pre-evaluation checklist for GrantFox contract issues.
- [Self-Review Checklist](self-review-checklist.md) — check your own PR
against this guide before opening it.
- [Screen Test Matrix](screen-test-matrix.md) — required test types per
screen.
- [Accessibility Checklist](accessibility.md) and
[UI State Catalogue](ui-states.md) — the concrete bars a UI change is
measured against.
116 changes: 116 additions & 0 deletions docs/self-review-checklist.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
# Self-Review Checklist

## Purpose

A short, actionable checklist to run through **immediately before opening a
PR** — not before payment evaluation. It exists so contributors can catch the
same gaps a reviewer would catch, before a reviewer ever sees the diff.

This is not a replacement for the
[Evaluation Readiness Checklist](evaluation-readiness-checklist.md) (the full
pre-payment-evaluation bar) or the
[Contributor Self-Assessment](contributor-self-assessment.md) (the form you
paste into the PR description). Use this checklist first, as a fast pass over
your own branch; use the other two afterward, when you're ready to write up
evidence.

## When To Use This Checklist

- Right before you push your branch and click "Create Pull Request."
- Again after any substantial change to an already-open PR.

## 1. Feature Completion

- [ ] Every item in the issue's Acceptance Criteria is implemented, or its
absence is explicitly disclosed in the PR description.
- [ ] The change matches the issue's Expected Behaviour, not a partial or
adjacent version of it.
- [ ] Every new interactive element is actually wired up — no button that
renders but does nothing when pressed.
- [ ] No unrelated files, renames, or formatting changes are bundled in.

## 2. Tests

- [ ] New or changed behaviour has a corresponding automated test, or the PR
explains why a test isn't applicable.
- [ ] Both the happy path and at least one negative/edge case are covered.
- [ ] `npm test` passes locally on the latest commit.

## 3. CI

- [ ] `npm run typecheck` passes.
- [ ] `npm run lint` passes.
- [ ] `npm run api:check` passes if the change touches
`src/types/pocketpay-sdk.d.ts` or `src/sdk-stub/` (see
[SDK API Compatibility](sdk-api-compatibility.md)).
- [ ] No required check is skipped, muted, or left failing on the latest
commit. If a check is failing for a reason you can't explain, see the
[CI Troubleshooting Guide](ci-troubleshooting.md) before opening the PR.

## 4. Screenshots Or Recordings

- [ ] A screenshot or short recording is attached for any change to visible
UI or user flow.
- [ ] Every state that changed is shown (e.g. loading, error, and success —
not just the happy path).
- [ ] The media shows the actual final result, not a mockup or an earlier
draft of the change.
- [ ] Screenshots contain only Testnet or dummy data — no real keys,
balances, or personal data.
- [ ] If the change is non-visual, the PR says so instead of leaving this
section blank.

## 5. Device Or Emulator Verification

- [ ] The change was actually run on a physical device or a simulator/
emulator — not verified by reading the code alone.
- [ ] The app loads without a red error overlay or crash after the change.
- [ ] The exact flow described in the issue was exercised end-to-end
on-device, including any error or edge-case states the issue calls out.
- [ ] If the change is platform-specific (iOS or Android only), it was
verified on that platform rather than assumed to work from the other.

## 6. Documentation

- [ ] Any contributor, user, or architecture documentation affected by the
change is updated, or the PR states why no update is needed.
- [ ] README links, code comments, and examples touched by the change still
match the current behaviour.

## Examples Of Incomplete Submissions

Each of these has shipped in a repo looking finished while failing one of the
sections above:

- A "Copy Address" button that renders correctly in a screenshot but isn't
wired to the clipboard — passes review 4 (screenshot) and fails review 1
(feature completion).
- A validation fix confirmed only by reading the diff, never launched in a
simulator — fails review 5 even if the logic is correct.
- A new screen with no entry in `__tests__/` or `tests/` — fails review 2
regardless of how polished the UI looks.
- A multi-state fix (e.g. locked / matured / withdrawn) that only handles one
state, with the other two silently left broken — fails review 1 unless the
gap is disclosed.
- A PR description with an empty or missing Screenshots section on a change
that clearly touches UI — fails review 4 by omission.

See the [Meaningful Change Guide](meaningful-change-guide.md) for the fuller
set of examples and the reasoning behind them.

## Final Check

- [ ] I completed sections 1–6 above for this specific PR, not from memory of
a previous one.
- [ ] I would be comfortable if a reviewer checked every box above against my
diff directly.

If any box is unchecked, treat the PR as not ready to open.

## Related Guidance

- [Contributor Self-Assessment](contributor-self-assessment.md)
- [Evaluation Readiness Checklist](evaluation-readiness-checklist.md)
- [Meaningful Change Guide](meaningful-change-guide.md)
- [CI Troubleshooting Guide](ci-troubleshooting.md)
- [Accessibility Checklist](accessibility.md)