I review a lot of code.
Reviewing code takes most of my days, on most days. Of course this is the kind of thing you want to hear when you also know that I work at Boeing. I work on factory software, manufacturing & quality systems. Even a minor miss on our part will inevitably lead to meetings with FAA officials, IT directors, and then we won't be able to make cool new features for a while. This in turn makes me sad, and probably ruins my whole quarter. It's important to me, and I take it seriously - more than my colleagues wish sometimes.
On a typical day I'll have a dozen merge requests across our various projects and microservices, spanning multiple languages, frameworks, and homegrown tools. We orchestrate these into the best internal software product, run by the best software team in the company (my opinion, but I try to be objective).
A good code review has to cover a lot of things. Not just individual structure, but also how it affects the overall architecture. Not just patterns used, but if the patterns were used correctly. Not just the tests, but if the tests are beneficial. Not just the components, but if they're well designed. Not just the variable names, but if they actually work mentally, if they're spelled correctly, and if they make sense in context.
Unfortunately, what tends to happen is that my first glance at the highest level concept for a merge request gets stuck on the lowest level details:
- You misspelled compoent.
- You nested 3 levels of ternaries.
- You left a cheap "this sets a variable" comment.
- You didn't capitalize the correct letter.
It's piddly stuff. They shouldn't make these errors but they do. Frequently.
I hate to call out these mistakes. I really do. It takes my time and wastes my day, delays the merge and impacts the schedule, and it frustrates the dev that wrote it. I try to overlook it. I'll try to let one go, but once I find two, I know I'll find five more.
And here's my real issue - I'm the technical lead - the senior developer. I need to consider the highest level parts. The architecture implications. The data management plans. And I get stuck on the lowest level parts. The language and comments. The petty things.
Am I the only one?
This code doesn't compile!
Oh it passes the transpiler or compiler, and it passes all the tests in CI. The demo went great. But this doesn't compile for me. ðŸ§
My Head First books say that I should "be the compiler." I repeat it to myself like a mantra. I believed it when I first read it. Now I'm stuck with it. ðŸ§
If I can't compile the code in my head, I can't execute it in my head to decide if it works, and think through how it works for the context of the system. The brain compilation error blocks further analysis, so it will take an entire 2nd review before it can pass the code review.
So there you go. I can't compile it if there's a bad comment or misspelling. So again, I ask: