Code review your own code

Every engineer should code review their own code before submitting it to others for code review. It's a valuable opportunity to look through the code you're about to ask your peers to accept and make any final changes. I do this every time I put up a pull request and it often results in changes.

The key here is to try to put yourself in the shoes of your future code reviewer, who probably does not have the same context you had when making the change. There are a few questions I usually ask myself:

Are my changes self-explanatory?

I try to keep my PRs very short; short enough that they can be summarized with a single title. With few exceptions, I consider it an anti-pattern if I need a description for the PR. If that's the case, usually my PR has gotten large enough to be worth splitting or the context that would be in the description is significant enough to be worth documenting in the code itself. Future maintainers should not have to pull up my PR to understand the context behind a complex decision. Code comments are a great place to put the reason behind a decision, but not much else.

Did anything change about how I would write this code?

Between the time I started a change and submitted a PR, my understanding of the problem/code context has usually improved. Often this results in new insights into how I could structure the code differently to be more SOLID, DRY, or maintainable. I often end up changing function/class names during my second pass at the code to be more self-explanatory. This being said, don't fall into the trap of trying to perfect your code. It'll never be perfect and that's fine; just make sure it's going to be easy to maintain, which mostly means easy to delete.

Are there any changes not relevant to this PR?

Sometimes there will be a change I did not mean to commit. The PR preview is a great place to check for this.

Am I following all the guidelines for my team?

The most obvious thing to check for is whether there is anything someone else is going to request changes on - whether that's code style, security, naming convention, or any other guideline. Save your teammates the hassle of telling you to fix something by catching it first and they will be much happier to review your PRs.










Comments

Popular posts from this blog

Crossing the Rubycon

The many benefits of working in small increments

Building up tests via context and let in Ruby