One Rule to Build Them All

Every business is different, every codebase has debt that needs to be worked around. Any given project has different success criteria (budget vs. cost vs. scope).

Whenever I read an engineering blog, there is normally some law or rule on offer which I have to carefully evaluate against the codebase I am responsible for.

There is however one rule which I will always follow on any project I work on:

If it doesn’t break your CI/CD pipeline, you don’t care about it

Or alternatively:

If you care about something, make something fail the build

This ‘rule’ makes a key assumption; You are running some form of CI/CD (the build) which can fail or pass. That, and the build pipeline should be a gate that stops a PR being merged and the given changes being released to production if it fails.

The most obvious example is your project’s automated tests (because you have tests right?):

If you care about unit tests, make unit tests fail the build

If those tests don’t fail your build, you can release with failing tests. If you have failing tests then you have broken features. And if you have broken features in production, then you didn’t care about it because you didn’t make the check fail the build.

This same logic works for many other tools an engineer can use:

• Static Code Analysis (e.g. PMD)
• Secret Scanning (e.g. trufflehog)
• Code Style (e.g. spotless)
• …

If someone commits a file indented with Tabs and not Spaces and your code style mandates Spaces, that commit should not be able to get onto main without something failing somewhere.

If someone commits code which introduces a SQL injection vulnerability (which SAST tools can detect) and you don’t prevent that commit from getting to production, then you don’t care about SQL injection.

Importantly, any of these normally blocking checks must be able to be bypassed for any given reason. More importantly, any exclusion must be codified:

commit 091c52859f38a4bb6e1448f68e46ce75a765c639 (HEAD -> main)
Author: Phillip Whittlesea-Clark <pw.github@thega.me.uk>
Date:   Fri May 10 20:30:06 2024 +0100

    Adding skipTests as its 4am and I need to fix this production incident

diff --git a/build.sh b/build.sh
index ce747d2..04a9f1f 100644
--- a/build.sh
+++ b/build.sh
@@ -4,4 +4,4 @@ set +e
 
 SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
 
-mvn clean install
+mvn clean install -DskipTests

Whether it’s an annotation, an ignores file, or a change to a build file, you should have a record of who and why the rule was ignored.

I spend most of my days working on Java codebases built using Maven; here is my current selection of plugins which fail the build:

• Maven Enforcer Plugin stops the build from including JUnit, Mockito (and other common test libraries) at compile scope.
• Spotless enforces formatting of all Java files using the palantir code formatter.
• Checkstyle checks that documentation on classes/methods/fields exists (and my favourite check, AvoidStartImport).
• PMD checks for programmer mistakes like missing breaks in switch statements.
• SpotBugs uses static analysis to look for bugs in Java code.
• JaCoCo tracks coverage data and will fail the build if line coverage is less than 85%.
• Find-Sec-Bugs is an extension to SpotBugs which checks for security vulnerabilities.
• Old GroupIds Alerter catches the use of dependencies which have been relocated to another Maven Group/Artifact.

tldr; If you care about it, make it fail the build.