3

I have a small team where I'm largely the only person who reviews what goes into production and makes sure that the code introduced doesn't contradict business logic, no bugs etc.. I know a lot of this can be improved with tests and we often use them, however I see tests at most complementing the human code review (what was tested, how it was tested will for instance always need a review of the tests and the code under test).

However, this all means I've become a bottleneck and a big part of my job has become about reviewing code which is a bit soul-sucking after a while. I'm sure there's many ways to spread the load (mainly I want to include everyone else in the review processes) but I am not sure how to start because I am more senior and have more experience with our system, and I'm not sure what I should be aiming for.

Would be happy to hear suggestions to have a better idea of what I should be aiming for, and what the best way to get there is.

Improve this question
8
  • 3
    "I want to include everyone else in the review process" -> pair programming
    – jonrsharpe
    Apr 4, 2019 at 12:18
  • @jonrsharpe: Actually, "everyone" is more like mob programming. In mob programming, unlike pair programming, the person with the keyboard is not allowed to think. She simply types what the "mob" tells her. The mob all look at the code on a beamer.
    – Jörg W Mittag
    Apr 4, 2019 at 13:14
  • @JörgWMittag I'm aware of mob programming, I'm not suggesting that everyone review every piece of code, I'm suggesting that pairing gets everyone reviewing some of the code, that they're working on with their pair.
    – jonrsharpe
    Apr 4, 2019 at 13:43
  • 1
    this question is discussed at meta
    – gnat
    Apr 5, 2019 at 12:52
  • 1
    I dealt with this by having my devs review the code the same time I did. The benefit is that they learned what I expected, and learned what to look out for when reviewing. Over time I watched them progress where they saw the same things and made the same comments that I did, which also improved the pre-review code quality as well. At that point I skipped doing most reviews. Essentially I mentored them. Personally I think that all of the devs should be doing reviews, it will teach them a lot.
    – Robert Hanson
    Apr 6, 2019 at 12:51

2 Answers 2

Reset to default
5

There are a number of ways to ease/share the burden:

Tools

You don't say what languages you're using but there are many tools out there that can do basic analysis for you such as StyleCop, ReSharper, FxCop etc.

Code coverage

For well tested code, coverage percentage should be in high 80s at least.

Peer review

Even if you're still the gate keeper, you could get more junior members to do reviews and get rid of the more obvious mistakes thereby reducing your work load.

Show & tell

Rather than spend/hours and days reviewing code up front, get them to run it up and explain their approach.

Pair programming

Some places have abandoned code reviews altogether in favour of pair programming. While you may still wish to have a final review, this can stop bugs getting into code from the outset as well as reducing the amount of code that will require review.

TDD

TDD focuses the mind as to what is actually required. Bells and whistles add code which in turn are a source of bugs.

Improve this answer
1
  • marking this as the answer since it's the most complete one.
    – Jad S
    Apr 7, 2019 at 10:25
1

I see tests at most complementing the human code review

Hmm out of everything you say this stands out to me the most. Maybe there is a nuance to your meaning here, but always remember that the idea is to produce working software, not well coded software.

I would suggest the first thing you do is write down the rules that you are reviewing the code against. ie

  • Unit test coverage 90%
  • no gotos!
  • blah

Once you've done that anyone can do a code review fairly quickly. Maybe you can't capture everything you want to enforce in a set of rules, but at the very least a large proportion of the work can be 'outsourced'

Secondly, I would suggests you stop doing code reviews completely. Let the team decide what the rules should be going forward and have a free hand in changing them, enforcing them or ignoring them as they feel works best for them.

Maintain the quality of your product by ensuring that non-functional requirements are put in when new features are suggested.

This is a far stronger control than a code review. If a page has to load in < 3 seconds, it HAS to load in < 3 seconds. If all SQL must be in SProcs so that it can be audited, then thats a business requirement.

But be sure to check that these are actual requirements of the business.

Improve this answer
2
  • 2
    While I agree that having a set of rules is a good starting point, the most important criteria for passing code review is "can the code be understood by someone other than the person who wrote it?". The only way to test that is to have someone else look at it.
    – Pete
    Apr 4, 2019 at 14:07
  • @Pete: that sounds like a good rule for a code review checklist.
    – Doc Brown
    Apr 6, 2019 at 9:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.