Or how to get looks-good-to-me on your pull request seven times faster
This is a generalized version of a checklist, I’ve created for my colleagues at Wayfair. Feel free to fork it and adapt to your team’s needs.
-
Setup your environment to have immediate feedback on your changes:
- Setup ESLint plugin in your editor.
- Setup Prettier plugin in your editor.
- Setup TypeScript or Flow in your editor.
- Setup stylelint in your editor.
-
Read your team’s coding standards and style guides.
- JavaScript code is following team standards.
- All code is WCAG Level AA compliant.
- Semantic tags are used where possible instead of
div
s andspan
s (headings, paragraphs, lists, etc.)- See Semantic HTML Tutorial for some examples.
- All interactive elements (links, buttons, form elements) are keyboard accessible and have visible focus states.
- Tab order of all interactive elements follows a logical pattern, usually their position on the screen and order in the DOM.
- All non-standard elements have appropriate ARIA roles.
- All interactive elements have accessible labels.
- For example, add accessible text using the VisuallyHidden component or something similar.
- All non-text content has a text alternative.
- For example, images have appropriate alt texts, clickable icons have titles, and videos have captions.
- UI looks good on any screen size (mobile, desktop, etc.).
- UI looks good with 200% page zoom.
- CSS code is following team standards.
- No hardcoded colors, font sizes, whitespace, breakpoints and z-indices.
- Always use design tokens instead.
- No style overrides of any component library components.
- No unnecessary CSS, ideally there’s no CSS at all.
- For example, prefer to use primitive and layout components instead of custom styles.
- Add tests for new functionality.
- For example, add unit tests for tricky code and regression tests for bugs.
- Make sure that the documentation and comments are up to date after your changes, document any new APIs.
- Read the ticket one more time and make sure everything is done as requested.
- Do a self review: carefully read all the code before opening a pull request. Think what kind of issues a reviewer may find in your code, what kind of questions they may have, what’s not clear. This alone can save you several review iterations and days in review.
- Having a coffee break or lunch before doing a self review could be a great idea to help you to look at your code with fresh eyes.
- Take notes, mental of physical, on what kind of comments you get in code reviews — you’ll notice that they are often repeated. You can save lots of time by making sure you won’t get any of the comments that you often receive.
- Hint: Use GUI Git clients for better experience like GitHub Desktop or PhpStorm.
- No lint warnings, type errors or test failures.
- Hint: run these checks locally:
npm test
. - Hint: many warnings can be autofixed, check the documentation for your ESLint plugin.
- Hint: run these checks locally:
- No new errors and warning in the browser console.
- All code is formatted with Prettier.
- Add screenshots or GIFs for any UI changes. This will help the person reviewing your code to understand what you’ve changed and how it works.
- If your team uses a particular template for pull requests, fill it. Otherwise at least make sure you have:
- the user problem you are solving;
- acceptance criteria of the ticket;
- testing you have done or plan to do before release;
- any pull request that are dependent on this one, or any tickets on which this pull request depends.
- Ask to people to review your code:
- a person who knows the domain well and can spot bugs in the business logic;
- an expert in the technologies you’re using who can help you improve the code quality.
- When you’re done with the changes after a code review, do another self review of the code and write a comment to notify the reviewer, that the pull request is ready for another iteration.
- Review all the review comments and make sure they are all addressed before the next review iteration.
- Make sure you don’t have similar issues anywhere else in your pull request.
- If you’re not going to follow any of the code review recommendations, please add a comment explaining why.
- Avoid writing comment like "done" of "fixed" on each code review comment. Reviewers assume you’ll do all suggested changes, unless you have a reason not to do some of them.
- Sometimes it’s okay to postpone changes — in this case you’ll need to add a ticket number to the pull request and to the code itself.
- It’s okay if you don’t understand some of the code review comments. Don’t hesitate to ask questions in the pull request, or ping the reviewer directly.
- JavaScript for impatient programmers
- Understanding ECMAScript 6
- Washing your code: write once, read seven times
- React docs (it’s one of the best resources)
- The Beginner’s Guide to React
- Advanced React Component Patterns
- A Short Guide to Screen Reader Friendly Code
- Accessible to all
- Accessibility Tips for Web Developers
- Assistive Technologies I Test With
Improvements are welcome! Open an issue or send a pull request.
Artem Sapegin, a frontend engineer at Stage+ and the creator of React Styleguidist. I also write about frontend at my blog.
CC0 1.0 Universal license, see the included License.md file.