In the article “A good pull request: what is it and how to do it?” I focused more on reviewer role, but a submitter role is also very important in the reviewing process of a Pull Request.
What do you need to be a good submitter?
Let's take a look at it a bit closer.
You are the main reviewer
Review your code as carefully as if you were reviewing someone else’s code. Try to predict problem areas.
Check and test your code before creating a Pull Request
Don’t force others to test your code for you. I worked with a programmer who didn’t run any tests and didn't run his code at all. He explained that he gets paid for writing code, not for testing it. So not many people liked him…
Make a checklist for yourself
Did you check all the items from it? It’s enough to have some small checks like “Have I deleted all lines of commented out and unnecessary code?” or “Have I checked there are no unused imports and variables?”. If you want some inspiration, please check my article “Before the Pull Request: Quick Checklist for Submitter”.
Ask yourself critical questions
“Is it easy to maintain this code?”, “is my code have a resistant to crash?”, “is there any security issues in my code?” (Yes, there are still those who send passwords in plain text with a get request)
Don’t touch your Pull Request too hard during the review process
Wait until it will be reviewed and then make changes and fix comments in the PR. Otherwise it might be hard for a reviewer to finish a review round.
Open Pull Request when the code is written for 30-50%
If you develop a big feature and want others to see the progress, you can open Pull Request when the code is written for 30-50%. Don’t be afraid that the code isn’t finished yet. Your colleagues can tell you about architecture problems and suggest the necessary design patterns. Just mark that this PR as WIP (Work In Progress) in the PR's title.
Try to make small Pull Requests
It's more convenient to review small PRs (300-400 lines) and don't miss anything critical.
Check the code using automation tools
Linters, formatters and different checkers are your friends here.
Use pre-commit hooks
Consider to take a look at
pre-commit, so you won't forget to run your automation tools.
Write unit tests
Always. Don’t be lazy. They can point to problems immediately.
Always respond to comments in Pull Request
Even if they are “fixed” comments. If you can’t fix something or you don’t need a fix, explain why in the comments. If you and your colleague made some decisions during lunch, write this decision in the Pull Request so that others can see it too.
Let your colleagues know about your fixes
“Hey, I fixed up your comments, could you take a look when you have time please?”
Fight for solutions you really believe in
If you think your solution is better than the one suggested, fight for it! But please be polite, listen to other side, consider risks, try to understand why another person is disagree and use links to articles which proves your point of view.
Don’t take comments personally
Think of every comment as a opportunity to your professional growth.
Code review is a very important part of the team‘s life. It is impossible to downplay the significance of this process. And you as a submitter is an important part in this process.
Do you use something from this list? Or do you have something that is not here? Please, share it with me!