Skip to content
HomeWorkBlogResume
Blog

What 10 merged PRs taught me about large codebases

How I find my way around repos like Mozilla accounts, Apache Airflow and Cal.diy, and the review that taught me the most.

September 20, 20264 min read
0

My first merged PR into Mozilla was five lines. Finding the file took longer than writing them.

Between February and September 2026 I had 10 pull requests merged into other people's projects: six in Mozilla accounts (fxa), two in Apache Airflow, and one each in Cal.diy and Brave's Search MCP server. None of them required understanding most of the codebase. All of them required finding the one place that mattered and convincing a stranger my change was safe. This post is about how I do both.

Start from what you can see

The first bug was small. If you had no backup email, the Mozilla accounts settings page said "None" in English, even for German or Japanese users. A translated string for it already existed. Nothing was using it.

I did not read the repo. I searched for the exact text on the screen, followed it to the component, and read the test file next to it, because tests tell you what code is meant to do. That fix (#19929) was merged in February.

The same approach worked on a harder one later. In Japanese, Chinese and Korean, settings buttons were rendering one character per line (#20724). The cause was flexbox: buttons default to flex-shrink: 1, and CJK text can break between any two characters, so a squeezed button collapses into a vertical column. The fix was to stop the shrinking and tell the browser not to break CJK words:

css
/* keep-all stops CJK from breaking between characters without nowrap,
   so long Latin strings can still wrap on spaces. */
button.cta-primary,
a.cta-primary {
  @apply shrink-0 break-keep;
}

I used break-keep instead of whitespace-nowrap on purpose: a long German label still needs to wrap on spaces.

The review that taught me the most

Next, I opened one PR with everything I had found: a typo, a Sass build change, and a spot where session tokens were filtered with a hardcoded 28-day expiry marked with a FIXME, even though the auth server already had the real setting, tokenLifetimes.sessionTokenWithoutDevice. My editor also reformatted lines I never meant to touch.

The maintainer was direct. "Do not make any formatting changes to lines that aren't changing," twice. The build change was for code they were already replacing. And my session fix was, in his words, "technially still hardcoded, but just a static member now". He was right.

The PR went stale. Instead of patching it again, I closed it and split it:

  • #20800: the typo, one line.
  • #20801: the auth server sets SessionToken.sessionExpiryMs from its own config when the database initializes, with unit tests for the expiry filter in the shared package.

Setting it once at startup kept the shared model free of auth-server imports. The cost is that the model now depends on startup order, which I called out in the PR. The review on #20801 was "Looks good to me. Thanks!" It was merged on September 18. The code in the original PR was not really wrong. It was hard to review, and I had not understood that counts as a defect.

Bugs that hide in the quiet paths

Several of my fixes were in code that only runs when something is turned off or missing.

In fxa's auth server, disabling StatsD registered a stub object with increment, timing and close. Redis metrics called this.metrics?.histogram(...). Optional chaining guards against a missing object, not a missing method, so with metrics disabled the call threw. The fix (#20799) moved the stub into a createNoopStatsd() helper that implements every method, with a test for its surface.

In Cal.diy (#29673), the push notification test ping hardcoded https://app.cal.com/ as its click URL, which is wrong on every self-hosted instance. It now uses WEBAPP_URL.

Sometimes the fix is the documentation

My Brave Search MCP server PR (#348) changed one line. The image search schema told agents to "combine this parameter with offset to paginate", but the Image Search API has no offset. Agents reading the schema kept inventing one. When the reader is a model, the schema description is the API.

Waiting on someone else's CI

Apache Airflow taught me patience. My change (#72432) made an empty plugins/ folder log a quiet debug line instead of a summary that read like a failure, and made real import failures log a warning with the file list. Tests passed locally, but nothing ran on GitHub: workflows from a first-time contributor's fork sit in action_required until someone with write access approves them. The reviewer approved it "pending green CI" but could not start CI either. I posted the exact pytest command and output on the PR, asked in the Airflow Slack's #new-contributors channel, and waited. It was merged in September.

Takeaways

  • Search for the exact text you can see, then read the tests next to the code.
  • One concern per PR. When a reviewer asks for a split, do the split.
  • Look at the disabled, missing and headless paths. They get the least testing.
  • Put the reproduction and the exact test command in the PR. It is what reviewers need to say yes.
  • First-time contributor CI may need a human to start it. Ask early, in the right channel.
0
All posts

SreeCharan Desu

Engineer at Manuscripts · Open to remote

HomeWorkBlogResume
Chat

© 2026 SreeCharan Desu. All rights reserved.