You’ve carefully placed all the code bricks into place, laying the foundation for your feature. Now you’re ready to commit, push, and open a pull request. But are you actually ready? Here’s my personal checklist I use before opening a PR. This method helps me catch mistakes before a review, critically review my code, and save the team valuable time.
This is a lengthy, but comprehensive list of what I’m looking for and thinking about when I review my own code. The speed of checking your code against this checklist should become second nature to you with more experience.
Mindset Shift
You’re now reviewing your own code. Forget that you wrote it and that you know how it works. Put on the dispassionate lenses of a PR reviewer to make sure that your code is correct and easy to understand. Then put on the ignorant lenses of a theoretical new developer on your team and take a fresh look at your changes. Is your code also readable?
For each file that has changed, I like to run through my code twice: once before I commit and again in a PR draft. It may be surprising, but running through it twice in this way has allowed me to catch mistakes, or even rethink decisions that I’ve made.
Code Changes
- Are there any obvious errors, perhaps highlighted by an IDE? Unused imports or variables?
- For each method that you’ve modified, review its parameters. For every parameter, run through the list of possible values. Does each possible value allow the method to behave in an acceptable way?
- For example, if a Java method has one integer parameter, what would the method do if this integer was zero,
Integer.MIN_VALUE,Integer.MAX_VALUE, or an integer in the expected range of values? - Are there sufficient unit tests covering these scenarios?
- For example, if a Java method has one integer parameter, what would the method do if this integer was zero,
- For each method that you’ve modified, is it still easily readable and understandable on a high-level by someone new to the code base?
- Does the method name still make sense after your changes?
- Can whitespace be added or removed to make it more readable?
- Consider adding comments where code is efficient, but hard to read.
- Is the method handling a lot of different types of tasks (e.g. validating parameters, parsing values, querying the database, mapping one object to another)?
Consider creating specialized private methods to handle each type of task to improve readability. Avoid creating methods too eagerly, such as one-liners.
- Is my code formatted to the standards my team has set?
- Have I run all the unit tests?
- Are there any failing unit tests that highlight a scenario that was broken by your changes?
- Are there any broken unit tests that need updating to match new requirements?
- Could any logging be added to improve traceability in other environments such as prod?
Configuration Files
- Are the configuration changes updated in other environment files so that they also work on non-local environments?
- Will these configuration changes break someone’s local development setup?
- Is there a way to prevent this, such as updating the README, or writing a quick script to set up someone’s local environment?
Documentation
- Have I documented my changes appropriately?
- Have I updated my ticket as needed after my changes?