Code Review for the Solo Dev

I will start this article, as I did with the previous one in the series.

Nothing can substitute a great team. But a real warrior has to be able to rely on thyself if needed.

A few people nowadays, work on a team of one.

People that are subject matter experts and the very best on their fields (and I am the last human on earth who can give them any professional advice).

People that have just founded, solo, their own company (or their company partners are not technical).

Or people who work always remotely, and in a vastly different time zone compared to the headquarters, where getting help from other colleagues might be difficult.

Code review, the milestone of minimum acceptable software quality

As Jeff Atwood(the founder of StackOverflow) points in his article Code reviews: Just do it

I believe that peer code reviews are the single biggest thing you can do to improve your code. If you’re not doing code reviews right now with another developer, you’re missing a lot of bugs in your code and cheating yourself out of some key professional development opportunities. As far as I’m concerned, my code isn’t done until I’ve gone over it with a fellow developer.

If such an experienced and successful engineer supports that boldly the importance of code review, even if you disagree, you have to at least give it a try.

Let’s move on with the most important areas, in my opinion, that the solo engineer should focus, during reviewing their own code.

Do the automated builds and automation tests pass flawlessly?

You don’t have either? Too bad, there are no excuses given that services like gitlab and bitbucket offer free CI services nowadays. And no excuses as well, given that unit tests are easier than ever to write, given the friendliness of the ubiquitous unit test frameworks.

In case you have both and Santa brought a present for being good, failing any of the automated processes, shall ring you lots of bells. But if not, it is not a sign to settle.

Have you written new tests?

No? Really? I am so disapointed. :(

I will quote a paragraph from Django’s contribution guide

When we say “PEP 8, and must have docs and tests”, we mean it. If a patch doesn’t have docs and tests, there had better be a good reason. Arguments like “I couldn’t find any existing tests of this feature” don’t carry much weight–while it may be true, that means you have the extra-important job of writing the very first tests for that feature, not that you get a pass from writing tests altogether.

TDD is hard but having unit tests and testable code has tremendous benefits.

Think about your code might affect existing functionality (regression)

This is an area where having a great test suite will save your arse one day. It always depends on the coverage you have in your suite and given most people have less than 100%, your intuition and analytical thinking will be of great help here.

Are the diffs sensible?

Have a look using any diff tool, to see if the changes made, make sense.

If you are working on a security issue, is a change on foo.css necessary? If you want to enhance a form, do you need to change 3 configuration files?

Probably not. Be vigilant, when it comes to diffs and it will pay you back. Pretend that someone else asks you, why you changed a specific line and be ready to explain thoroughly to your fantastic friend.

Run a security analysis

Have you introduced any security holes accidentally? And I mean obvious stuff, like passing un-sanitized input to the database. There are plenty of security static checkers around you can pick the ones that fit more to your job.

Also, checking that no sensitive information is being logged is crucial.

For example, if the software is meant to be deployed to a PCI-DSS compliant company, then no card number shall be logged. And is very easy to forget that, while logging all the args of a function.

Speaking about logging, are you (under|over)logging?

Under-logging will cause you to rip your hair off when a sneaky bug come to the surface. Especially if you had logged some incidents around the affected area.

Over-logging can slow down your code, increase exponentially the log files' size and cause to you search a needle in the hay when inspection is needed.

My suggestion is to use log levels, where the default one can be easily changed. In that case, you can increase the logging output when investigating an issue and have more clear logs when you don’t. Unless the state of investigating is way more frequent than the state of working flawlessly.

Is the code taking care of extreme cases and invalid parameters?

What will happen in your system if the network goes down in the middle of a huge DB update or if I give you my name as my telephone?

This is a bit overlapping with the security checks laid above.

Have you updated the tickets, the documentation, and the changelogs?

Not really a functional requirement, but necessary if the software you are working on is not for personal use (even then, it is useful to keep track of your actions).

Usually, this includes the following:

• Close the ticket in the issue tracker and add the relevant pull request link/complete diff link, if it is not done automatically (eg Jira-bitbucket integration)
• Close the pull-request, if you were working on a branch (highly recommended even for solo teams)
• Update the changelog
• If required, amend the API/user documents and push them to the users

Some more points that are worth considering

• Does the code comply with the company’s standards? Even if it is a solo company, where you are the owner? Dirty code is tempting sometimes.
• If you changed a function, did you remember to amend its comments?
• Can a complex expression and magic values be extracted to a(n) (inline) function and a constant, respectively
• Do the new classes/functions have proper documentation?
• Do the packages/classes/functions/variables have meaningful names?
• Did you accidentally have commented code or duplicate code in place, which pollutes the readability?
• If your new code is exposed to huge data volumes or traffic will it scale? Are there any quick wins to improve the situation?
• Will you be able to work on this functionality in two years, without wasting too much time re-familiarizing?

Last but not least, it is way more difficult if you do it alone

Just like in testing, it is difficult to judge yourself hard and this is common with most humans.

During the last years, there are services where people give your code a second look, like pullrequest. I have not tried the service, but it looks like a good idea when being solo is the only choice and the stakes are high.

That’s all folks

Thank you for taking the time to read this article, I hope you enjoyed it. In case, you have a different approach to reviewing code, please share your wisdom.

Wishing you all the best for the year that is coming.