don't worry, it's probably fine

Off the beaten path: finding code to review

14 Nov 2019

nablopomo code review

I like code reviews, despite what I said yesterday.

One of my favourite things to do when I have a bit of time is finding a bit of code that hasn’t been touched for a while and see if there are any improvements I can make.

We called this gardening in a previous team, to capture the idea that code grows like a plant, and needs trimming and weeding in the same way.

I started off by persuading git to do what I wanted (other DVCS are available)

For every file that git knows about, ask it when the last commit was for that file, and then order the files by that date.

for FILE in $(git ls-tree -r --name-only HEAD); do
    LAST_COMMIT=$(git log --pretty=format:%ad -n 1 --date=raw -- ${FILE})

    echo ${LAST_COMMIT} ${FILE}
done | sort --numeric-sort --reverse | head

A good first cut, but with flaws.

It tends to throw up a load of metadata-ish files like .gitignore or gradle stuff, but we can refine the output of git ls-tree by pumping it into something like fgrep 'java'.

There are times when a change isn’t actually important - applying an automated linter for the first time could plausibly change every file in the codebase but those changes don’t have semantic meaning.

We can do better!

Who let the dogs out?

A slack bot is a piece of code that puts messages into our Slack channels - a new pull-request has been opened, an alert has gone off, and more.

We have a slack bot called Daniel the Manual Spaniel which tells us when bits of our documentation are due for review.

Each page of documentation optionally declares (a) when it was last reviewed and (b) how long until the next review.

The bot uses that metadata to decide whether something needs reviewing and post in Slack about it.

I really wanted something like this but for code. So I had a crack at it for java code.

Introducing … review

Attaching metadata to code

I puzzled about the best way to have something like we have in our documentation. In an ideal world it wouldn’t require any external processing, and could be implemented in pure java.

I decided on building a compile-time annotation processor.

Annotation processors can be used to generate code on the fly - you have access to metadata about the class and the annotations during compilation time. For this task

The following example code was last reviewed at the beginning of November and would be due a review two weeks later

@Review(lastReviewed = "2019-11-01", reviewIn = "2 weeks")
class ToBeReviewed {}

The annotation processor finds all classes with the @Review annotation and logs out whether a review is due.

During compilation, you would see something like this:

$ ./gradlew clean compileJava

> Task :compileJava
Code due for review: ToBeReviewed

Sweet! Now I have a way to tag when code needs a bit of attention.

It would be a small jump to wire this into an automated check that looks at build logs and similarly posts a message to a chat app, but this is left as an exercise for the reader.

I learned a lot about how java’s compile-time annotation processing works. I’ll be publishing this to Maven Central or Bintray soon so you can have a go yourself!


November is National Blog Posting Month, or NaBloPoMo. I’ll be endeavouring to write one blog post per day in the month of November 2019 - some short and sweet, others long and boring.