Ruth Evans

Posted on Jul 2, 2019

How to code review in a start up?

#help

I work in a small team of 5 developers and we've decided that we need to start doing code reviews which we definitely should have been doing already.
I have experience for doing code reviews in my previous job for a large company were we could afford to spend the time and people on such tasks which is not the case here.

Does anyone have any words of wisdom for code reviewing in small teams?
How do you make time to review code?
What do you do if there isn't a reviewer available who can understand that code or programming language?
How do you resolve disputes when the reviewers and the authors are the same technical level?

Top comments (10)

Rich • Jul 2 '19 • Edited

How do you make time to review code?
You make it part of the process. Allocate 15 minute tasks to every piece of work for code review. It can be formal or informal, just make sure you schedule it and make it a requirement of checking in any work.

"we could afford to spend the time and people on such tasks which is not the case here." isn't really an argument. If you are saying your company cannot make time for code review, re-phrase this as "We don't care about introducing technical debt into our product, or checking things meet requirements"

What do you do if there isn't a reviewer available who can understand that code or programming language?

It doesn't get checked in. If nobody can understand the code then checking it in will just create technical debt or reliance on the person that made it.

They should be able to at the very least tie the code to the original spec, or produce documentation explaining it. Even if the code reviewer isn't familiar with the specific language, I would expect most developers to be able to walk through a solution and at least understand the approach being taken.

How do you resolve disputes when the reviewers and the authors are the same technical level?
This is standard conflict resolution, both sides make their argument and as a team you decide the action taking into account the desired output and the pro/con of each approach. One important thing is to make sure nobody is starting work without an agreed requirement/output so it makes it easier. For pure technical dispute (i.e. you should have used library X), it should fall to the technical lead to have the final say, but ultimately it's a management decision of whether you refactor the work to take the most "desired" approach, or accept what has been done. So many variables.

Good luck! It should be an easy one to fight for - I am yet to meet a manager who won't allocate time when using the re-phrasing from above!

Ruth Evans • Jul 2 '19

Thank you, this is all really useful 😊

Karolis Jocevičius • Jul 3 '19

I actually don't see much need to overcomplicate this. Especially when you have only 5 people.

While I don't work in a startup, my team consists of 4 people. For all our projects, we have rules, set in version control:

'master' branch is protected and you can't push to it directly at all.
Pull requests must be approved (by anyone from the team)
CI checks must be successful

Also, preferably, code reviews shouldn't be too big, because then nothing ever will be reviewed.

That's it! Technical level doesn't really matter. Junior devs can both spot stuff and learn something by reviewing code. Time allocation shouldn't be an issue - send it for whoever is free at the moment.

Also remember that code reviews are not there for bug prevention, but rather for code quality improvement.

Phil • Jul 2 '19 • Edited

Does anyone have any words of wisdom for code reviewing in small teams?

Accept the views of each individual. Do not work against each other. By this I mean that it can quickly happen that critical comments on one's own code can be taken to heart, although they are only well-intentioned.
Write down a small checklist.
Always checkout the work.
Run tests.

How do you make time to review code?

Try to schedule it. Just like writing tests. I know the client does often not understand why you need tests or code reviews. Explain it to them well, so that they will understand it. For example they will get a better product and less bugs.

What do you do if there isn't a reviewer available who can understand that code or programming language?

Simply wait. I know time is money, but do you really want to push unreviewed code to your master branch? If someone in your team does not understand what you've written, take the time and explain it. You are a team and as I said "Do not work against each other". Try to get the same knowledge base. Work together and try to push each other. This is not a competition.

How do you resolve disputes when the reviewers and the authors are the same technical level?

Be honest with each other and with yourselves. Try to understand each other and stay reflected on yourself. I think the same "level" is not a problem. The most important thing is simply to discuss together and listen to the views of others! You can see it as a sort of "rubber ducking".

Hope this will help you and your team a little bit.

Ruth Evans • Jul 2 '19

This is a lot of help, thank you! 😊

Sandrina Pereira • Jul 3 '19

How to make time to review code

For me, code reviews are as important as writing the actual code. With your team, remember to include code reviews in your workflow, and set expectations about how long a code review might take.
I prefer to do code reviews first thing in the morning or after lunch before picking any other of my tasks. That’s what works best for me because my brain is still fresh without a previous code context to switch off from. Besides that, once I’m done with the review, I can focus on my own tasks, while the author can reevaluate the code based on the feedback.

What do you do if there isn't a reviewer available who can understand that code or programming language?

Remember to write documentation, it doesn't need to be a detailed page as MDN with full specs. I know that in a startup the time is short, so a simple, small document explaining the overall can be enough: Try to answer these questions:

The motivation behind,
Hight level approach
Include reference links (ex:: explaining a pattern or a known bug/workaround). When someone more familiar with the programming language looks at the code, that doc file will be gold to him/her.

How do you resolve disputes when the reviewers and the authors are the same technical level?

This one is tricky and it's where a lot can go wrong and destroy all the purpose of code reviewing. Based on my experience, the best thing you can do is to try to be flexible and put your ego aside. Learn to overcome feedback resistance and to accept trade-offs.

I've written an article about Bringing Healthy Code Review Mindset to your team some time ago. I hope it can help you and your team too.

Ruth Evans • Jul 4 '19

Thank you for your reply 😊 The article was really useful, especially writing a pull request template and making constructive suggestive comments instead of directive commands.

Sebastian Vargr • Jul 2 '19 • Edited

The way i have done it in small teams have always been treating PR's more like sanity checks. I find trusting your team is much better than scrutinizing them. :)

This is how the process i usually encourage is:

Each pr needs to be approved by 1 other developer
Keep PR's small, a code-review should be doable in 5-10 minutes and should not require the reviewer to test things, it's a code review, not a test.
ignore code-style as much as possible, install a lint-fixer, like Prettier, if it is a problem in the team.
if you lack a reviewer with the proper language skills pull someone else over and explain the code to them, it's works the same in terms of sanity checks.
Always side with whatever fixes the problem now, if there is a "better" way it can always be done in a followup PR. And if all else fails look for someone more senior and hear them out.
The lead/senior dev' on the team can always overrule above when the need arise.

I find doing it like this keeps productivity high and style debates at minimum.
Of cause, this approach is more suited for small contained teams where a occasional bug getting past developers and onto the testers is not a problem.

I also usually highly encourage developers to read up how to behave on PR's.
It is a sensitive situation, especially for juniors, and deserves to be treated with respect to avoid pissing someone off because of a stupid misunderstanding. :)