Pull Requests and Code Review

A pull request (PR) asks to merge a branch. It shows the diff, runs automated checks and collects review comments. Even solo developers benefit: the PR is a checklist and a record of why each change happened.

The flow

1. git switch -c feature/invoices
2. Commit in small, logical steps.
3. git push -u origin feature/invoices
4. Open the PR on GitHub with a clear description.
5. Automated tests run (GitHub Actions, covered in Automation & Scripting).
6. Review, discuss, push fixes to the same branch.
7. Merge, delete the branch, pull main locally.

A good PR description

## What
Adds PDF invoices for contract payments.

## Why
Clients asked for downloadable invoices for their accounts.

## How to test
1. Pay a test contract with card 4242 4242 4242 4242
2. Open Account → Payments → Invoice

## Notes
Amounts use to_minor_units so JPY is not multiplied by 100.

Keep PRs small

A 50-line PR gets a careful review in minutes. A 2,000-line PR gets "looks good to me" and hides bugs. Split big work into steps that each leave the app working.

Reviewing well

• Check behaviour first: does it do what the description says? Are edge cases handled?
• Then security: input validation, permissions, secrets, SQL built from strings.
• Then clarity: names, duplication, tests.
• Ask questions instead of giving orders: "What happens if amount is empty?"
• Approve when it is good enough and safe, not when it matches your personal style.

Merge strategies

Squash keeps main readable when branches contain many small "wip" commits.

Protecting main

In Settings → Branches, add a rule for main: require a pull request, require status checks to pass, and block force pushes. Now nothing reaches production without tests passing.

Check yourself

• What four things belong in a PR description?
• Why are small PRs reviewed better?
• Which branch protection settings stop untested code reaching main?