A checklist to go through prior to submitting a PR. A PR should be about the change that's being proposed. In order to focus on that it's good to make sure there aren't any trivial points that need to be tidied up before people can focus on the real changes that you've made. Running through this list before making the PR should eliminate most of…

Frontend PR Checklist

Things to check before submitting a PR.

Git

[ ] Have you definitely created a new branch for this PR using `git checkout -b <branch name>`?
[ ] `git add` all your changes to the branch.
[ ] `git pull` on the branch that you've based the branch on, and then merge that branch in to your branch.
[ ] Are there conflicts? Fix them now.

Feature/Bug

[ ] Check each of the AC in the story. Have they all been achieved?
[ ] Check the designs. Does the feature look like the designs? Have differences been noted?
[ ] Check the bug replication steps. Is the bug definitely fixed?
[ ] Check other likely problem areas. Have you introduced any regressions? Fix them.
[ ] Does the change affect the details in the README file? Update the README.

UI

[ ] The UI must be responsive. Are all components working asynchronously (eg no awaits)?
[ ] Things should fail gracefully. Are components handling their loading, loaded, and failing states properly?
[ ] API calls can be slow or fail entirely. Are slow/failing API calls handled by components well?

Tests

[ ] Are there tests for any new features? Is there enough coverage? Check.
[ ] Are the existing tests up to date with the code changes? Update any tests that should change.
[ ] Do all the tests pass? Fix any tests that are failing.
[ ] Does the feature work properly in all target browsers (Chrome, Edgium, Firefox, Safari)?
[ ] Does the feature work with network conditioning? Try it with 'Slow 3G'.

Code cleaniless

[ ] There should be no ambiguous variable names. Make sure it's clear what things are, especially in destructuring.
[ ] Is there any dead (commented) or unreachable code? Delete it.
[ ] Are there any inline styles? Refactor them out to CSS.
[ ] Are there any 'magic numbers' (values in code)? Refactor them out to a constants file.
[ ] Are there any secrets in the constants? Refactor them out to env vars.
[ ] If you've copy'n'pasted any code, update any old names or comments so they're relevant to the new code.
[ ] Is the linter complaining about anything? Fix lint errors (and warnings if possible).
[ ] Is this an opportunity to refactor? Check the code and see if there's anything to improve. Improve all the things!

PR

[ ] Link to the story in the project management app
[ ] Link to any other related PRs (eg API changes)
[ ] Write a useful description. Try not to just duplicate the story; describe what the change is, not what the change is doing.
[ ] Select any reviewers who need to check the code. Ask for more reviewers if there's only one.
[ ] If relevant add screenshots, or even a video.

Notes

The git section is just basic gitflow. When you pick up a new story (feature or bug) you need a branch in order to PR it in to develop or main. If you're making a quick fix it's easy to forget, so make sure you've made a new branch before you push anything. Also make sure you've added all the code to the branch before you PR anything because missed things could be lost (especially if you delete local branches to tidy up occasionally). Switching back to the original branch and doing a git pull, and then merging that branch in to your new branch, is simply a way of getting the latest code from the repo and adding it to your branch - this means that any conflicts can be seen on your branch, and you can fix them before they block the merge. This also means that small changes (eg whitespace) won't clutter up the PR and make it hard to tell what's really changed.

The Features/Bug section is about working with a project management app. It's important to check that the acceptance criteria have all been met and that the work matches the designs (if there aren't any AC or designs you shouldn't have picked up the story). Likewise for bugs - QA will run through the replication steps to check the change works, so do that yourself as well so the PR won't be rejected for not working. Lastly, changes to the code can always break things, so think about what the changes will have affected and check them as early as possible.

Working in the frontend means writing code that drives the UI, and the UI needs to be fast. The easiest way to make a UI slow is to wait for the network before updating things. Avoid this at all costs.

It's also important that the frontend fails gracefully. Bugs and network request failures will happen. If the app dies when something fails that's bad. Write code that expects things to go wrong and deals with it.

We should always be writing more tests. Test all the things. All new features should have automated test. Regressions and bug fixes should have tests so we know if they come back. Changes to legacy code should be treated as an opportunity to add tests.

All of our frontend code needs to work in the four major browsers (Chrome, Edgium, Firefox and Safari) and on mobile where the application is usable on mobile. This means testing in all the target browsers. We should also have Playwright tests to run real scenarios if possible.

Clean code is good code. All of our code is linted which picks up a lot of issues, but we should also be running some basic sanity checks of our own where appropriate.

Try to avoid single character variable names. They don't mean anything and I won't understand them in 6 months time. Avoid unnecessary contractions of things (eg use connection instead of cnnct) - they're easier to read. If typing longer names is a concern then configure your IDE to do autocomplete properly.

There should be no code in the repo that can't run. Commented code or unreachable code should be deleted. It's in the git history - you won't lose anything by deleting it.

Inline styles are hard to work with and promote duplication, so move them out to SCSS/CSS files.

Same for magic numbers. If there's a number in the code that could be repeated that's likely to end up the source of a difference in the future. Put the number in a constants file and then replace the number with the constant everywhere it's used.

Never put secrets in code. API keys, passwords, etc should not in the repo.

If you've copy'n'pasted some code in your change go through it carefully and make sure everything has been changed to be relevant to the new place where it's used. I don't want to see a comment for the previous place the code was used. It's just confusing.

Run the linter (it should run when you commit something). If there are warnings or errors fix them.

Every PR should link back to the project management app to make it easier to see what the change is supposed to do. Similarly every PR should link to related PRs if they've been made in other branches or other repos.

Write a useful description of the change that's been made. That doesn't mean duplicating the story - it means describing the changes that have been made in the code, not what the changes do. Don't be afraid of being verbose. It's better to overcommunicate what the change is than to leave the reviewer wondering.

Every PR should be checked by more than one person if possible.

Github lets you add screenshots, so add some. Be proud of the work you've done. Show it off. Make it really easy to know that the change has been made, has been demonstrated to work, and that it looks right.