DEV Community

Clara
Clara

Posted on

Do you double check your PR’s?

Do you ever read your own pull requests? Or are you more of a >that’ll do< uploader?

I received my first code review with a lot, like a lot of annotations, including typos that I could have caught myself - that was embarrassing.
Since then I have established a short checklist that I follow before I submit a pull request for a code review.

1 Double check every file that I submit

I give myself a nice long break before submitting the PR - if possible I come back to my code on the next day, just so I am the most distanced I can be. Since the person that will review my code will have a very different perspective than I do, This will give me the chance of reading my code as the reviewer. Helping catch any error or unclear line of code that I could not have seen before. Since the person reviewing my PR will have a different perspective than I, this gives me a chance of reading my own code from their point of view. This type of distancing will make it easier to catch errors and any unclear lines of code that I may have missed before.
Bonus: It helps with exactly knowing what you did in this PR in order to write a nice summary.

GIF with subtitle "I should have double checked"

2 Question variable names

While in the zone, I often don’t think a lot about naming my variables, although I’m not as terrible as just naming them x or array. However, Naming variables can be decisive in making your code readable, So therefore the pull request PR is a good opportunity to question my variable names. the choice of names I have given. Same goes for method names - here I take my time and think about if that method might be used in another scope or scenario later and therefore should be named differently.

3 Watch out for typos

Especially if you’re dealing with any kind of text, a transactional email you were drafting, or an automated flash message - double check your spelling.

4 Run a linter

If you haven’t done so, now would also be the time to run a linter checking your style.

Those points might seem obvious, but I am sure following them religiously has spared my reviewers some comments.

Let me know what you are checking before you submit a PR for review.

Oldest comments (7)

Collapse
 
utkarsh profile image
Utkarsh Talwar

That looks like a good checklist to go through before submitting a PR. We actually had a healthy discussion yesterday about what constitutes a "good" PR, as we're updating our company best practices for the same. One of them is that for new features, adding a GIF of the working feature is mandatory wherever applicable.

Collapse
 
clara profile image
Clara

I love the ideas of GIFS - when I was at the beginning of my career I was terrified to submit PRs, I guess that would have lighten my mood a little bit.

Collapse
 
totally_waqas profile image
Waqas

Always; plus, using another UI to review my code helps catching stuff I would have otherwise missed

Collapse
 
cbloss profile image
cbloss

I really do try to go over my own requests. That way I can check my own coding standards and try to catch any stupid logs I left in there.

Collapse
 
clara profile image
Clara

Oh yeah, how often do I miss a console.log somewhere.
What are your own coding standards?

Collapse
 
cbloss profile image
cbloss

I've left so many console.logs in code! Bah. For me, I mainly trying to avoid comments that make no sense. Especially when I changed the code to where the code is irrelevant.

We have two different sets of standards. One for the backend (which are PS-4 Symphony's coding standards) and one for the frontend (Airbnb's). They are trying to make it easier for people to read.

Collapse
 
dougaws profile image
Doug
  • If you are making a code change, you should have an associated unit test.
  • If you have any associated documentation, that should be reviewed by an editor (and spell-checked).
  • Linter/formatter