Article summary
Recently, I’ve been looking for ways to improve the code health of a project I’m working on. It’s a pretty big team, so things are moving quickly. While the codebase is in good shape now, little things can quickly spiral out of control.
As I was thinking of ways to protect the code health, I was faced with a predicament:
- On one hand, code health tools are of the utmost importance to keeping a project moving forward.
- On the other hand, we need to get features done, and an elaborate code health tool simply isn’t what the client requested.
The Best of Both Worlds
With these thoughts in mind, I looked for a way to compromise. What if there were a simpler alternative to a complicated, intricate code health tool–something more straightforward that could provide enough value to keep using it?
I wanted a solution that would work for multiple projects. Many of the problems that show up on my current project stem from a few known issues, but every project will have its own “gotchas” and quirks.
I also needed to be mindful of a few practices that this team likes to follow:
- Avoid leaving TODO comments.
- Be explicit about using TypeScript types. When it’s not possible, use the
shame
type (a wrapper aroundany
, and inspired by this post). - Avoid leaving
console.log
. - Favor
mapDispatchToProps(dispatch)
overmapDispatchToProps(dispatch, props)
to prevent unnecessary component re-renders.
My Process
With these parameters in mind, I set out to see how I could track down problems and provide a simple interface for developers to fix them.
Phase 1
The first step was as simple as searching for key phrases in the codebase. My tool of choice these days is ripgrep
(alternatively: grep
, ag
, or ack
), and it was pretty easy to crudely build some regexes that matched.
rg 'shame'
rg 'console\.log'
rg -i '//.*todo'
rg 'mapDispatchToProps.+\(dispatch.+[pP]rops\).+\{' # matches typed and untyped variations of mapDispatchToProps = () and mapDispatchToProps(), with props or ownProps as parameters
I wanted to limit the search to files that I care about:
rg -e 'shame' -e 'console\.log' -e 'mapDispatchToProps.+\(dispatch.+[pP]rops\).+\{' -i -e '//.*todo' `git ls-files 'src/*.js' 'src/*.jsx' 'src/*.ts' 'src/*.tsx'`
I figured those five minutes couldn’t help that much….oh, 123 matches? It could be worse. I knew I could fix some of those issues now and come back in a few days when I had more time. But my memory isn’t great, so I would probably forget to revisit the results and discuss my findings with the team.
Phase 2
The first step in sharing this with the team was adding a tool to the repository. One approach would be to create a new file, add the searches, make it executable with chmod 644 path/to/script.sh
, and move on. Another option would be adding it to a task runner.
It turned out that putting this in the package.json
to run as a Yarn task worked great for this project:
"scripts": {
"auditor": "rg -p -e 'shame' -e 'console\\.log' -e 'mapDispatchToProps.+\\(dispatch.+[pP]rops\\).+\\{' -i -e '//.*todo'",
"auditor:all": "yarn run auditor `git ls-files '*.js' '*.jsx' '*.ts' '*.tsx'`",
...
},
Well, that was pretty simple.
While Yarn isn’t a one-size-fits-all solution, there are many task runners that could do the trick. If your project doesn’t use a task runner, consider setting up a shared `environment` file to keep small utility functions in a central location.
My only problem at this point was remembering to come back and re-run this search every once in a while.
Phase 3
One possible solution to the write-and-forget problem is to add it to a pre-push Git hook:
files=`git ls-files '*.js' '*.jsx' '*.ts' '*.tsx'`
audits=`yarn auditor $files;`
if [[ -n $audits ]]; then
echo 'There might be some potential issues with the code that is being pushed. '
echo "$audits"
exit 1
fi
exit 0
However, all this output is pretty chaotic, and I don’t want clutter up my terminal every time I push. Alternatively, I could only check the files that have been changed:
while read local_ref local_sha remote_ref remote_sha
do
if [ "$local_sha" = $z40 ]
then
# Handle delete
:
else
if [ "$remote_sha" = $z40 ]
then
# New branch, examine all commits
range="$local_sha"
files=`git ls-files '*.js' '*.jsx' '*.ts' '*.tsx'`
else
# Update to existing branch, examine new commits
range="$remote_sha..$local_sha"
files=`git diff --name-only $range`
fi
audits=`yarn auditor $files;`
if [[ -n $audits ]]; then
echo "$audits"
echo 'There might be some potential issues with the code that is being pushed. '
exit 1
fi
fi
done
exit 0
(Please note: the template for this pre-push hook was modified from the official Git pre-push.sample).
My preferred method of sharing Git hooks is to commit them directly in the repository (not in the .git
folder) and have individual team members symlink those in their respective .git/hooks
directory. Otherwise, documenting the code and behavior is a viable alternative, but it can become out-of-date.
This example is pretty simple, but the tool does exactly what I wanted it to do. It shows areas of the codebase that could be cleaned up, and it discourages committing code with potential issues. Each step (however small) is immediately useful. Some tools can be built over the course of minutes, while others might take months or even years.
Lessons Learned
- Simply searching through the codebase for problematic terms in a repeatable manner can be immediately useful.
- Every project is different. If searching for certain terms will not benefit yours, consider creating a different tool.
- Helpers and project-specific tools can be incrementally built (and improved) as needed, without a large up-front commitment.
- Centralizing the location of tools and utilities makes them accessible to the whole team.
- It’s important to utilize the tools that you create. Automating can make it easier to use them.
- If you create something, share it with your team.