Adopting Coding Standards Within a Large Codebase

Over the past month here at ReadMe, we've doubled the size of our engineering organization (…psst) and one thing that we quickly realized was that each of us have our own coding styles. This unfortunately was starting to result in comments on pull requests like, “Indent this line a few more characters so it's consistent.”

Coming from a company that had a 10+ year old monolithic codebase, I am very well versed, with how quickly code can cruft itself if not properly maintained or monitored. We were already using ESLint here for code quality and static analysis, but ESLint can only go so far to keep your code looking sharp and consistent.

Enter, Prettier. Prettier is an “opinionated code formatter” and it does just that. It'll reformat a file you supply to match formatting rules like spaces vs tabs (see: opinionated), line length, comma usage, etc.

Investigation

One of the issues we immediately came to a head with when starting down the path of adopting Prettier within our codebase was that we didn't want to have one giant, silly, pull request. We neither wanted to do what a lot of projects online do and rewrite our entire Git history. That's a big yikes! We also don't care a whole lot about existing code matching our new coding standards, just that new code does.

Prettier, thankfully, offers two options that we could use immediately:

• --require-pragma
• --insert-pragma

--require-pragma will look for either a @prettier or @format docblock annotation at the top of files, and if present, will assert Prettier runs against that file. --insert-pragma does the same, but will insert that annotation into the file if it is not already present.

So for new files that are added to our codebase, we can require engineers here to add those annotations to the top of their file for Prettier analysis, but this unfortunately leaves us with a few problems:

• None of us are ever going to remember to add @prettier or @format to the top of new files, and we want to avoid a mass changeset with --insert-pragma.
• Changes to existing, un-Prettier’d, code is never going to get Prettier’d when we make changes.

How can we automate our piecemeal adoption?

Automating Our Adoption

For code that we've ported over, we can piggyback on ESLint with eslint-config-prettier and eslint-plugin-prettier to run Prettier assertions at the same time as our existing ESLint tooling in our build system.

What about old code that we're working in though? Unfortunately Prettier doesn't have a way, with --require-pragma, to also specify a list of other files so we need to hack up a custom solution. Thankfully, since all we care about in this instance is changed files we can use Git to our advantage here:

git diff --name-only remotes/origin/master...HEAD | awk '/(js|jsx)/{print}'

This command will diff your branch against our current master, and dump out a list of .js and .jsx files that you've changed. We can then pipe the output of this along to Prettier by way of xargs:

git diff --name-only remotes/origin/master...HEAD | awk '/(js|jsx)/{print}' | xargs -n 1 ./node_modules/.bin/prettier --no-config --single-quote --trailing-comma=es5 --list-different

If there are files that you've edited in your branch, this command will list out those that don't conform to Prettier standards.

One thing to note here is that we unfortunately need to duplicate our .prettierrc configuration in these scripts because our .prettierrc has requirePragma set to true. This automation process is entirely for the purpose of migrating old code up.

A similar command can also be run to include our found changed files in future Prettier assertions as well by utilizing --insert-pragma and --write:

git diff --name-only remotes/origin/master...HEAD | awk '/(js|jsx)/{print}' | xargs -n 1 ./node_modules/.bin/prettier --no-config --insert-pragma --single-quote --trailing-comma=es5 --write

Let's put a little bit of friendly UX around this by wrapping it up inside a nice Bash script:

#!/bin/bash
CHANGED_FILES=$(git diff --name-only remotes/origin/master...HEAD | awk '/(js|jsx)/{print}')

if [[ $1 == "check" ]]; then
    OUTPUT=$(echo $CHANGED_FILES | xargs -n 1 ./node_modules/.bin/prettier --no-config --single-quote --trailing-comma=es5 --list-different)
    if [[ $OUTPUT ]]; then
        echo $OUTPUT
        echo ""
        echo "Whoops! There are some files that need to be migrated over to"
        echo "Prettier. You can run'npm run prettier:migrate' to take care of"
        echo "that."
        exit 1
    fi

    exit 0
elif [[ $1 == "migrate" ]]; then
    echo $CHANGED_FILES | xargs -n 1 ./node_modules/.bin/prettier --no-config --insert-pragma --single-quote --trailing-comma=es5 --write
    exit 0
fi

echo "This script will facilitate us in slowly migrating our codebase over to"
echo "conform to Prettier. See https://prettier.io/ for more information."
echo ""
echo "USAGE"
echo "  ./bin/prettier-migration.sh <command>"
echo ""
echo "COMMANDS"
echo "  check      List files that have been modified in the last day that need"
echo "             to be migrated to our Prettier."
echo ""
echo "  migrate    Migrate files that have been modified in the day to conform"
echo "             to our Prettier."
echo ""
exit 0

Finally, adding ./bin/prettier-migration.sh check into our NPM test processes to will trigger our continuous integration builds to fail if new, and modified code, doesn't conform to Prettier.

Our codebase will now automatically, gradually over time, adopt Prettier standards!