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

Post a Comment

Popular posts from this blog

Crossing the Rubycon

I first learned Ruby about a year ago and it quickly became my scripting language of choice; a position I doubt it will ever lose, hence the name of this blog post. Here are some of the reasons that I prefer Ruby over Python, NodeJS, and PHP, for most use cases. Testing The primary reason I prefer Ruby is how great writing tests is. Tests in ruby can be incredibly concise and natural. The built in mocking, stubbing, and monkey-patching functionality is very powerful. I particularly love the way contexts can be built up with let(:...) to have very descriptive and concise tests. The philosophy If you look at a language like PHP, it has basically no philosophy behind it, at least not a consistent one. Hence you end up with all sorts of weird behaviors. For example, consider preg_match , which finds matches of a Regex in a string: preg_match() returns 1 if the pattern matches given subject, 0 if it does not, or false on failure. Gross .  Wait, where are the results of the regex match? // 
Read more

The many benefits of working in small increments

If you're a software engineer, you've likely heard of the Single Responsibility Principle  (SRP). I believe it's the single most important principle in software engineering. The simple explanation is that one object should do only one thing and do it well. Having a well defined single responsibility makes code easier to understand, change, delete, and test. The complexity of code grows exponentially with size. Two 100 line classes, with proper segregation of responsibility, are significantly easier to understand than a single 200 line one. SRP is the "work in small increments" principle for code, and the same exponential growth in complexity applies to much more than just code. Working in small increments shortens the feedback cycle, which is the best thing any organization can do to increase its velocity. Small Tasks Every task should be small - small enough to complete in less than two weeks, ideally less than two days. Small tasks are easier to write, understan
Read more

The two qualities of great software architecture

Great software architecture solves a present business need and is easy to change when business needs change. Software architects have developed a number of heuristics: patterns and principles that help to achieve these goals. A software engineer that applies but does not fully understand these heuristics may implement them in situations that run counter to the goal. To write great software, it's important to understand how each pattern and principle contributes to the goal. Entire books have been written on most of these subjects. This is intended to be a brief overview, not an exhaustive list or explanation.  Solving a present business need Most users could look at an application and tell you whether it solves a present business need, aside from some of the more subtle nuances. For example, an application may solve one need but is not secure, which is usually one of the business requirements. Some of the criteria that may roll up to this metric, depending on your business: User ex
Read more