This is a post in our continuing series about Agile in Practice and the last detailed post describing where we started.
Automating Code Checks: Why We Did It
We had all the right ingredients to set up automated code checks early:
- We could implement automated code checks quickly (cost was low)
- We had several developers working on fast changing code bases (value was high)
- We did have some better stuff to do - there always is other stuff to do - but that could wait a little while to to prevent issues down the line
Automated Code Checks: An Example
Let's show a quick example of what automated code checks do. We'll use a python example. But keep in mind most languages have automated checkers like this, and the output will be similar.
Imagine we have the following code:
import os
import asyncio
from rich.prompt import Prompt
from nuve.<redacted>.models import *
from nuve.<redacted> import *
from nuve.helpers import cprint, random_str
...
user_id = Prompt.ask(f"Choose the user [bold][{default_user_id}]", default=default_user_id)
Pretty standard stuff, but there are some issues with this code. Let's try to add it to our codebase via a github commit.
shell-prompt % git commit -am "example commit of above code"
black....................................................................Failed
- hook id: black
- files were modified by this hook
reformatted nuve/.../file.py
All done! ✨ 🍰 ✨
1 file reformatted.
flake8...................................................................Failed
- hook id: flake8
- exit code: 1
nuve/.../file.py:1:1: F401 'os' imported but unused
nuve/.../file.py:5:1: F403 'from nuve.<redacted>.models import *' used; unable to detect undefined names
nuve/.../file.py:6:1: F403 'from nuve.<redacted> import *' used; unable to detect undefined names
nuve/.../file.py:7:1: F401 'nuve.helpers.random_str' imported but unused
...
isort....................................................................Failed
- hook id: isort
- files were modified by this hook
Fixing nuve/.../file.py
The automated code checks show us exactly what is wrong, and fixes a lot of stuff for us. We can walk through each individually.
Failure 1: Python black formatter
The black formatter automatically shortened that line that was too long, making it more readable.
user_id = Prompt.ask(f"Choose the user [bold][{default_user_id}]", default=default_user_id)
user_id = Prompt.ask(
f"Choose the user [bold][{default_user_id}]", default=default_user_id
)
Failure 2: Flake8 Rules
Issue 1:
Flake8 told us we had some libraries that were imported but unused, and where to remove them.
nuve/.../file.py:1:1: F401 'os' imported but unused
nuve/.../file.py:7:1: F401 'nuve.helpers.random_str' imported but unused
That's helpful. Less imports means faster build times and less clutter in the code.
Issue 2:
Flake8 also tells us to be clearer about what we're importing. Also helpful. It's much easier for future developers to understand where objects in the code come from if they're imported explicitly.
nuve/.../file.py:5:1: F403 'from nuve.<redacted>.models import *' used; unable to detect undefined names
nuve/.../file.py:6:1: F403 'from nuve.<redacted> import *' used; unable to detect undefined names
We hid the imports for security purposes and to keep this post short, but further down Flake8 tells us exactly which imports were not imported explicitly, making this fix simple.
Failure 3: isort Import Rules
isort automatically re-arranged our imports alphabetically using standard import patterns:
import os
import asyncio
from rich.prompt import Prompt
from nuve.<redacted>.models import *
from nuve.<redacted> import *
from nuve.helpers import cprint, random_str
import asyncio
import os
from rich.prompt import Prompt
from nuve.<redacted>.models import *
from nuve.helpers import cprint, random_str
from nuve.<redacted> import *
Wow! That's a lot of stuff that helps people to read my code in the future. The fixes are really fast
It would also tell us if we have issues in our code like:
- Undefined variables
- Libraries that we are using that weren't imported
- Incorect syntax
- etc.
Which is awesome. Can't tell you how many times a linter has caught an undefined variable bug in my code before I ever ran it. That's really, really helpful.
How We Set Up Code Checks
We had 3 codebases:
- An ABAP codebase for SAP work
- A Python codebase for API work, and
- A JavaScript (TypeScript) codebase for our web interface
And each had its own rules.
ABAP Codebase
We have a whole post on this available here. If you're setting up abapLint, that's the place to start.
Python Codebase
We started with just the Black auto-formatter and a custom script to auto-format code that one of our developers had on hand. We later switched to using Pre-Commit with Black, Flake8 and isort so that we would catch all sorts of relevant issues prior to ever sharing code with others.
JavaScript Codebase
We used esLint with the following settings, which matched our codebase settings.
"env": {
"browser": true,
"es2021": true,
"node": true
},
"extends": [
"eslint:recommended",
"plugin:react/recommended",
"plugin:@typescript-eslint/recommended",
"plugin:@next/next/recommended"
],
"parser": "@typescript-eslint/parser",
"parserOptions": {
"ecmaFeatures": {
"jsx": true
},
"ecmaVersion": "latest",
"sourceType": "module"
},
"plugins": [
"react",
"@typescript-eslint"
],
"settings": {
"react": {
"version": "17.0"
}
},
...
We originally implemented them using just a github runner to enforce before merging into the main codebase, but eventually moved them to Pre-Commit as well so we would learn about issues as quickly as possible.
Conclusion: Checks Create a Clean Codebase
Setting up these checks may seem like a lot, but the outcome has been great for us. All of our code has been written in the same style. We don't have a lot of unused variables, definitions or imports, and so on.
All of this helped to make future maintenance and development easier. We'd recommended to almost all teams.
Final Note: When to Delay or Avoid Implementing Automaded Checks
There are some circumstances where you don't want to implement automated code checks immediately. Here are a few.
Reason 1: You Have One or Two Developers Maintaining a Slow Moving Codebase
This is the "Low benefit" reason for avoiding automated checks.
Let's say Jane and Harry are the only two people looking at an old codebase. It's changing slowly, and it mostly does what it needs to do.
They probably understand the code pretty well, so it doesn't need massive readability improvements immediately. The business probably doesn't want to change it that much, otherwise they'd be asking for change.
In this world, things are fine. No need to create problems where none exist. But if Jane and Harry all of a sudden need to start making lots of changes to the codebase, automated checks might start to make sense.
Reason 2: Setting Up Automated Checks Will Be Super Time Consuming
This is the "high cost" reason for not setting up automated checks, and it's usually a reason to delay the process or ask for outside help.
Imagine nobody on your team knows how to set these checks up, so a) your automated check rules will take a long time to implement and b) they might not be that helpful once implemented.
If the above situation applies and a team member really wants to do it, then great. They can figure out a way to reduce the costs of implementation. They can find team member or consultant who knows how to set up checks like these or try it out on a personal project first to build implementation skills and perspective.
Reason 3: You Have Something Better to Do (Cost/Benefit Wise)
This is the "high opportunity cost" reason for not setting up automated checks, and it is a reason to delay implementing automated checks.
Even though we set up automated checks, we didn't set them all up immediately. That's because we knew we had a few rules to follow that would prevent most issues. And we frankly had better things to do - building our early products - than making sure our code was perfectly tested.
That said, there will come a day when the team will really start to crave cleaner code. And since an ounce of prevention is worth a pound of cure, you don't want to wait too long to start enforcing clean code in an automated fashion.