Skip to main content

PRs

This document describes our best practices with Pull Requests that contribute to CRUK repos and helps make reviewers' lives easier.

info

The following content is recommended.

Remember

Reviews Are Conversations - A well-prepared PR isn't just about the code—it's about starting a productive conversation with your teammates. When you make it easy for others to understand and review your work, you're contributing to a culture of quality and collaboration.

The extra 10 minutes you spend preparing a thoughtful PR can save hours of back-and-forth, reduce context switching for reviewers, and make everyone's day a little easier!

Rationale​

Pull requests are a critical part of our work. They provide a standardised interface for code reviews, progress marking, issue tracking, and simplified testing suites and deployments.

Creating Your PR​

Write a Clear Title​

The name of the PR you create or generate should have some meaning. Ideally the PR name will be the same as the commit message header. See the Commits page for details and examples.

Write a Helpful Description​

Include a description, link to ticket, and test information. If applicable, also include any helpful links such as links to related tickets and decisions, UI references, GitHub issues or external resources. In order to help reviewers and make it simpler to see what has changed, you may also add screenshots for UI updates or any link or blog that you referred to. Note: For bigger/unscoped/unexpected decisions and changes (e.g., architectural changes to support user story, technical decisions due to limitations)these should also be documented in the appropriate places (i.e., on the Ticket, in docs/ or the repo, wiki or Architectural Decision Records, or ADRs).

Example:​

❌ Poor Description​

fixes stuff
see ticket
JIRA: https://jira.cancerresearchuk.org/browse/CR-1234

✅ Clear Description​

## Description/summary
Add product reference data API to support admin console dropdown selectors.
Users need to select from valid products in fulfilment and email edit pages.

JIRA: https://jira.cancerresearchuk.org/browse/CR-1234

## Changes
- Created new GET /api/products endpoint
- Returns filtered list based on user permissions
- Cached for 5 minutes to reduce DB load

## Deployment Considerations (if applicable)
- Database migrations required
- Feature flags needed
- Environment variables added
- Dependencies updated

## Areas of Concern (if applicable)
- Where you specifically want feedback
- Parts you're unsure about
- Performance implications

## Testing
- [x] Unit tests pass (npm run test)
- [ ] Integration tests added
- [x] Manual verification with admin console
- [x] Screenshots below show dropdown working

Make It Visual (When Relevant)​

  • UI changes: Include before/after screenshots
  • API changes: Show request/response examples
  • Complex logic: Add diagrams or flowcharts

PR Sizing Guide​

The golden question: Can a reviewer understand this in 30 minutes?

Some rough guidelines:

  • Sweet spot: 200-500 lines of changes - easy to digest
  • Getting chunky: 500-800 lines - still manageable but consider splitting
  • The monster: 800+ lines - probably worth breaking up (but we get it, sometimes it happens!)

Context matters more than line count:

  • 50 lines of complex algorithm? Might need extra explanation
  • 1000 lines of mostly config changes? Could be fine with good description
  • Multiple unrelated fixes? Definitely split these up

When it makes sense to split:

  • Feature can be broken into logical chunks
  • Database migrations + code changes
  • Multiple bug fixes that aren't related
  • Refactoring mixed with new features

When it's okay to keep it big:

  • Generated code or large config updates
  • Tightly coupled changes that don't make sense apart
  • You've added extra context and explanation to help reviewers
Remember

These are guidelines, not rules. Use your judgment and prioritise making it easy for reviewers to understand!

Help Complex PRs​

Sometimes, big complex changes do happen, for those we want to:

  • Break into smaller PRs when possible
  • Add inline comments explaining tricky parts
  • Include links to relevant documentation or discussions
  • Consider recording a short video walkthrough

Before You Request Review 🚦​

Self-Review Checklist ✔️​

  • Review your own PR first - Read through every change as if you're seeing it for the first time. If you have Copilot enabled for your repo - use it! It's a great tool to use in tandem with reviewing your PR.
  • Check the diff - Look for unintended changes (debug code, console.logs, formatting issues, typos). Idealy this should alreayd be automated in your repo with eslint and prettier.
  • Test your changes - Run tests locally, verify functionality works as expected. Check that automated tests on CI are passing and address any failed ones.
  • Update documentation - Add/update README, comments, or docs if needed

Is Your PR Ready?​

Ask yourself:

  • Can someone understand what this PR does without asking me questions?
  • Have I included enough context for the reviewer to verify the changes?
  • Would I be able to review this PR if someone else wrote it?

Review Process​

  1. Assign appropriate reviewers - Include both senior and junior engineers when helpful
  2. Add QA when ready - Mark with "Ready for Test" label
  3. Respond to feedback promptly - Address questions and comments quickly
  4. Update description if scope changes - Keep it current throughout the review
  5. If using a squash and merge strategy, add a description when the PR is being merged.

Here you can find journey for contributing Github repo and mentioned above steps.