3

I work with a small team of software engineers. Each one of us handle different projects without much interaction. We all report to the same supervisor who more or less understands the requirement of all the projects each one of us are working on. He used to be the only person that would review our code.

Seeing that it didn't make sense for all code to be reviewed by him alone, he now wants us to review each other's code.

The problem is aside from using the same programming language, I am completely oblivious of what the other projects are doing. And since most of my colleagues have been working on their respective projects for some time, it's a fairly large code base for me to dig into and familiarize myself to the point of being able to give useful reviews.

So how should I proceed in reviewing code from an unfamiliar code base?

0

1 Answer 1

7

Your reviews are useful already.

Why?

As the neophyte in the code base, you know nothing... Which is incredibly valuable.

Asking the experienced and seasoned expert Why? is one of the most valuable tools you can bring to the review.

  • The Expert has to articulate their domain knowledge in order to share it with you. Perfect to organise into developer documentation.
  • The Expert has to justify the design to you. How hard they have to justify it indicates either its absurdity, or its sheer difficulty.
  • As a Neophyte in the domain you can more easily see issues that the expert has long since acclimatised too.

Standards

Perhaps the easiest way to become effective is for everyone to commit work similarly.

E.g. a commit can only contain:

  • One consistent refactor.
    • Such as renaming all classes starting with XYZ to start with ABC.
    • Changing a family of function signatures in exactly the same way to remove/add/move an argument.
  • One defect fix/partial defect fix.
    • A single unit test highlighting the issue + the exact lines to fix the issue.
  • A feature/sub-feature
    • A small set of tests describing the new feature being added, the new lines of code to support it.
  • Removing a feature/sub-feature
    • Removing just the lines of code and associated tests.

At the very least this keeps the review focused, and largely self-contained.

If several commits must be reviewed in order to accomplish a piece of work, at least the pieces are themselves easy to work with, and compose to form a story:

  • First we need to cleanup some naming
  • Reorganise some files
  • We've identified a bug, and here is the fix
  • The Foo object needs some new behaviour
  • Adding a new implementation of IObj
  • Configuring the IoC
  • The business behaviour is X and adding two lines in the controller enables it.

Learning

Unfortunately, you will have to learn the other codebase.

The best approach is to ask to be given a small feature to implement. This allows you to explore and practice in the code base, and better yet the actual expert verifies your code.

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