Never approve design docs

And don’t treat them as launch checklist items

(Like this article? Read more Wednesday Wisdom!)

Because of my distinguished graying beard, I often get asked to review design docs for a new service or a significant new component of an existing service.  Sometimes this happens in a “formal” design review meeting, sometimes people just shoot the design document to me on Slack with a request for a review. I love doing that and the people who are wise (or perhaps unwise) enough to ask me for a review, typically get dozens of detailed comments ranging from criticism on their data types (e.g. overuse of strings) and the lack of a formal data model (really, a few informal JSON blobs is not enough) to an explanation of the Two Generals Problem.

If you do not know what the Two Generals Problem is, stop now and read that Wiki page first. I promise that that will be the most wisdom you will get from this week’s Wednesday Wisdom. The rest of this article isn’t bad, but in terms of sheer wisdom per character, that Wiki page squarely beats me (this week at least).

The concept of a design review is so powerful that many companies have processes for it, often in the form of regularly scheduled design review meetings with attendance by senior engineers and a schedule of what is going to be reviewed when. Many engineering organizations make it compulsory to go to a design review for every non-trivial design document.

That last bit might sound great, but there is some downside risk, which is that it can turn the design review into a checklist item.

Here’s what I mean by that: The lifecycle of a software project starts with an idea and ends with a launch. Design reviews validate the quality of the original idea and its proposed implementation and launches are typically validated using an abbreviated launch checklist or by doing a full operational readiness review. Usually one of the items on the launch checklist or readiness review is whether the design for the launched feature has been subjected to a design review.

That is a good thing to know for sure, because if some ideas or assumptions were never reviewed, that is a risk that you might want to know about.

In the late 1980s I worked for a bank that had rebuilt the input component of their massive batch system. Unfortunately, a core assumption of the new design was that you could write to a VSAM Key Sequential Data Set (don’t ask) from two jobs (processes) at the same time, which was unfortunately not supported by the operating system. During testing things worked well enough because of the small size of the test set and the low level of concurrency, but during its first night in production the new system crashed spectacularly, corrupting data and prompting a rollback.

The presence of a design review checkbox on the launch checklist can result in sending a never-before-reviewed design document to a review very late in the game just so that the team can tick the box on the checklist.

This is mostly useless.

The whole purpose of a design review is to get useful input on your ideas very early so that you can incorporate any feedback in the design before you are going to build the damn thing.

Or, in some cases, whether to build it at all. I recently sat in a design review where the communis opinio was that we should not implement the proposed solution.

If you go to design review with a system that, for all practical purposes, has already been built and where really the only purpose of the review is to tick off some box on a launch checklist, you are wasting everyone’s time. I typically do not participate in a design review once I figure out that that is the deal.

By the way, this doesn’t mean that there is no value at all in a review at that stage of the game, but the goal of that review is different. Instead of giving input on design choices and finding ways to improve the design, the reviewer(s) should focus on finding launch blockers in the design.

My proposal is to reword this item on the launch checklists out there to something like: “Was the design sent to a design review early and were comments on the design weighed appropriately and, where applicable, incorporated into the final version?”. To be followed by: “If not, was the design reviewed (post-development) for critical launch blocking deficiencies?”

Another important aspect of design reviews is the status of recommendations by the reviewers: Are these “binding” edicts for change or are they instead suggestions that the team can incorporate or not as they see fit? In other words, are design reviews there to offer friendly professional advice, or are they there to approve the design?

I strongly prefer the former.

The reason for this is, and it’s very atypical for me, modesty. I have a ton of knowledge and experience that I can bring to bear on a problem, but at the end of the day I get to spend just a few hours on reviewing the proposed design. I think that is enough to sometimes make a few good suggestions and it is definitely enough to warn people who are about to make a mistake that I made in the past, but it is not enough to authoritatively approve or reject a design (or parts of it).

At the end of the day I want everyone to be in charge of their own destiny and if the team, weighing what they know and what they heard from me, thinks that a particular direction is the best one, then more power to them. They have to build it and they are responsible for making it work. If a design review is an approvals process, it is almost by definition a train wreck.

More than a year ago I wrote a Wednesday Wisdom article about that particular aspect of processes: I never approve of anything (except of you).

By approving, I would take responsibility for the correctness of the design and I can’t really do that without a lot more work than a typical design review takes. In that world, CYA demands that the reviewers are more thorough and that would take more time. Eventually, the capacity of the reviewers would become a bottleneck, slowing things down. It would also give the development team an easy way out if things go south: “Well, we asked these old wise people here and they approved.” None of this is optimal.

The review panel in design reviews should by the way not be restricted to old wise people; I want design reviews to be an open forum where everyone, junior to senior and everyone in between, can come and participate.

As I said, I want teams to be in charge of their own destiny, and as a result I want them to come to me for a review only if they are truly interested in what I have to say. If they are not, that's maybe not great, but let’s not waste one another’s valuable time.

Another aspect of this is that I like design reviews to happen very early in the process. I am totally fine with a rough set of ideas and a few pages with some haphazard diagrams. The more effort has gone into the design, the more solidified the design is, and the less room there is for changes. The earlier you come to me, the easier it is on everyone to halt the process in its tracks or change direction. It’s okay if I see a design two or three times in the course of development. If anything, it makes the final design much easier to review if I have seen the preliminary drafts and provided input on it.

The goal of the design review is to improve the design. That means that the design needs to be brought to the reviewers by a team that is interested in the review comments and at a moment in time that it is still feasible to make changes. Doing it any other way is just process for the sake of process, ticking off a box here or there in order to pay tribute to the checkbox gods. By coming to me for a review, you are not handing over responsibility for the correctness of the design; at the end of the day it is your job to make sure that your system works. I’ll gladly pitch in, but I will not take responsibility away from you and that means that I cannot take final authority away from you (since these need to go hand in hand).

As I like to say during reviews: “I think this is going to work, but I reserve the right to change my mind if, after launch, it turns out that it does not 🙂”