Chapter 8: PR Review Hell

It was going to be a good day.

Cramer arrived at the office early, half-skipping up the stairs. His code was ready. The feature worked. He was done. In his head, he was already spreadsheeting the raise: if he kept his lifestyle fixed and put 100% of it into a high-yield savings account, in 11.3 months he'd have enough to quit and finally build something of his own. Sooner if he stuck to store-brand lentils.

He opened his laptop, made a few final tweaks to the PR description ("Simple addition to status bar – data already sourced, tested and verified"), and hit "Create Pull Request."

150 lines changed.

He smiled. Small PRs are easier to approve. Cramer expected the review to be quick.

It was a tiny feature. The code was clean. In his mind, he'd ship by lunch, maybe take the afternoon to catch up on his backlog of productivity podcasts — he was three episodes behind on how to stop procrastinating.

Instead, what followed was not a review.

It was a campaign.

A slow, demoralizing war of attrition fought over the course of fourteen long days — each bringing new demands, new opinions, new rules he'd apparently violated by writing code the way any sane person would.

Day 1.

"Hey, just noticing some test failures on CI."

He opened the logs. Dozens of red flags. Most of them from test files he didn't remember existing. Apparently, some brittle test cases were still asserting the exact HTML structure of a now-nonexistent <div> hierarchy.

Who the hell writes a test like that? He git blamed the worst offender.

It was him. Two years ago. Younger, more naïve. The kind of dev who thinks using #mainContainer .content-wrapper > p + span:first-children selectors is "precise."

He fixed the tests, then got pinged again.

"Looks like some lines aren't covered by unit tests?"

Of course. The coverage bot had found the gaps. Including:

• a one-line onKeyPress that just checked if the key was "Enter" — and called a clickHandler that was already covered by tests
• a service function that wrapped axios.get()
• a data-testid string literal that dynamically generated the test ID

Fine. Snapshot tests for all of them. His tests were now testing that React worked. But the red dots were gone.

Day 3.

A comment popped up:
"Heads up: this CSS animation doesn't work on IE11."

IE11. Still officially supported for "select enterprise clients."

He rewrote the animation in JavaScript. It looked worse and required 70 extra lines of logic. But now it worked on a browser that even Microsoft didn't support anymore.

Day 4.

A new comment thread opened: "Performance review"

He braced himself.

"You can remove one <div> here."
"Use <section> instead of <div> for semantics."
"Add fetch priority hints for icons."
"Why eagerly load this child component? Use React.lazy."

The component was 12 lines. But sure, lazy-load it.

He complied. Then someone else chimed in:

"Why not prefetch the data on hover?"

"It only shows when users actually click the icon", Cramer replied.

"We prefetch where possible."

So he added on-hover prefetching. But then someone else said that was wasteful. He switched to on-click.

Then back to hover again.

By the end of it, it lazy-loaded on hover, fetched on click, and displayed after a timeout. Utterly overengineered, but technically the most performant.

Day 9.

"What if the endpoint fails?"

Cramer: "It never has — and if it did, half the dashboard would be down anyway."

"Still."

He added a retry mechanism.

"Now you're retrying too often."

He added exponential backoff.

"What about distinguishing 400s from 500s?"

He added specific error messages.

"And logging?"

He added logging.

"And localization for error messages?"

He briefly considered logging himself off a cliff.

Day 11.

"What if the absentee number becomes longer than 3 digits?"

"It caps at 100% by definition. You can't have more absentees than employees."

"Unless they fire people mid-week. Then technically the number of absentees could exceed the remaining headcount and the percentage would go over 100%, maybe even reach 1,000%. Let's prep for that."

The conversation swerved again. Someone had a strong opinion about number formatting, so he added locale-aware number formatting with thousands separators and fallbacks for browser language inconsistencies.

"What if the translated label is too long? Use defensive CSS."

He sighed, added text-overflow: ellipsis and gave the text container a max-width of 200px just in case. For two numbers.

Day 12.

"Instead of <p> and <div>, could you use the Text and Flex components from the Design System?"

Sure. He replaced clean HTML with components that rendered the same, just with more ceremony.

Then someone flagged 6px spacing. Apparently, it should be tokens.spacing.s.

Then another person asked:

"Why are you using count === undefined ? 0 : count instead of count || 0?"

Cramer didn't feel like debating code clarity versus error safety. He changed it.

"Actually, the original was clearer."

Thread length: 23 comments.

Day 14.

He had done it all. Every nit. Every tweak. Every preference disguised as a standard.

The PR had grown from 150 lines to 1,200. Most of it had nothing to do with the actual feature. He felt like a prisoner rewriting his own confession, trying to guess what the inquisitor wanted to hear.

He was ready.

And then:

"Why use <span /> here? Please replace with <Span /> from the Design System."

Cramer stared at the comment.

He replied: "<Span /> doesn't support onMouseEnter."

"Then open a ticket. The Design System team will add support next quarter."

He closed the PR.

Opened a new one from the same branch.

Requested review from Anil.

Anil didn't read it and approved immediately.

Then Cramer pinged Hugo, a friendly dev from another team. Hugo saw that someone else had approved it and added "LGTM" with a thumbs up emoji.

PR approved.

He had won.

All it had taken was 2 weeks, his soul, and whatever joy he once found in writing code.

Next Chapter: Microaggressively Yours