Prevent accidental commits with DO NOT SUBMIT

One of the teeny tiny features I miss from the Google development stack is something called DO NOT SUBMIT. Here is what it is and how to replicate it in your projects.

Say you are authoring a code change and want to send it out for early review, knowing that there are some loose ends that must be addressed before checking the code in. For example, you are aware that significant tests are missing, or you know that a dependent required change hasn’t yet rolled out to production. Regardless, the code is in sufficiently good shape that you want feedback to later shorten the submission time.

Obviously, you can add comments to the Pull Request (PR) description noting all of these circumstances, but if you are like most multi-tasking developers and end up with many parallel outstanding PRs, it’s easy to miss those comments later on. When that happens, you risk merging changes before they are ready, requiring a rollback or a rushed follow-up. And when I say “you”, I mean either you or any other person in the team that has the powers to merge approved changes: once a PR is approved, it’s typically only a button press away to merge it, and it is tempting to click it to “help move things forward”.

The question, then, is: how can we prevent such accidents? One possible solution is to mark such “incomplete” commits in a way that tells the version control system that they are not ready. At Google, one way to accomplish this is by adding the magic DO NOT SUBMIT words anywhere in the commit. When these are present, the version control server refuses the change even if it has been approved by all required parties.

The beauty of the DO NOT SUBMIT mechanism is that it’s extremely simple and low friction: you add some text and block the change from submission; you remove said text and unblock it. Given this freedom, though, this trick only works when you trust your peers. (And if you don’t trust them… you have some serious cultural issues.)

Why trust? Because if someone sends you a code change for review and they include a note of the form DO NOT SUBMIT: Wait until Monday., you have to trust that they will wait as promised if you approve the change earlier than that. In other words: as long as the code change looks good and you agree that waiting until Monday is the right thing to do… then you ought to approve the change.

I used this feature very frequently to tag commits with messages like: DO NOT SUBMIT: Wait for Bazel release X.Y.Z to be pushed. or DO NOT SUBMIT: Wait until commit X reaches cluster Y and then check if Z is working. and these caught mistakes more than once.

Things get really interesting when we go one step further and use this feature to tag parts of the code that require cleanup. Note that I said above that the magic keywords have to appear in the commit. I talked about the commit’s description… but the description is just one part of the commit. What if we put these words anywhere in the contents of the PR? By doing that, we can tag areas that we know are incomplete simply by putting a comment next to them, which is super useful when rapidly prototyping or iterating on some piece of code.

“How does this differ from the common TODO?”, I hear you say. Well, typically, TODO markers can and are checked in: they indicate an area of the code that you think deserves attention, but you are choosing not to address it because it’s not strictly necessary or because you did not have the time. And that’s OK. In the case of DO NOT SUBMIT, you are indicating that the area of the code must be changed prior to submission no matter what.

I used this feature countless times too, and I liked it so much that I even replicated it in some of my most recent open-source work. In the case of EndBASIC, I added a lint check that looks for these words in the source tree and aborts the CI run whenever they are found. Given that I have no extra reviewers in this personal project, having very strict lint checks has already saved me a few times from submitting garbage to the main branch of the repository.

If you want to implement this feature on your own, start by creating a new code health workflow in your CI system and then add a simple grep invocation to look for the magic DO NOT SUBMIT marker.

But how do you check in a change that searches for these words? If the words are present in the grep invocation, won’t they trip over the check itself and prevent it from being submitted? Of course! Which means we have to use tricks to avoid the literal words from appearing in the checks. For example, here is the little snippet I use in EndBASIC’s linter:

check_do_not_submit() {
    for f in .* *; do
        [ "${f}" != . ] || continue
        [ "${f}" != .. ] || continue
        [ "${f}" != .git ] || continue

        if grep -R '[D]O NOT SUBMIT' "${f}"; then
            echo "Submit blocked by" "DO" "NOT" "SUBMIT" 1>&2
            return 1
        fi
    done
}

Note how neither the grep nor the echo calls contain the DO NOT SUBMIT marker literally. They both apply some trick or another to stub it out.

Also note that this linter check is super-simplistic: in this implementation, I’m ensuring that the whole project doesn’t contain these magic words… but those whole-project checks scale poorly with the size of the project and the number of contributors. You might want to consider only looking for these keywords in the contents of the PR’s diff if that is a problem in your situation.