DEV Community

Enjoy Kumawat
Enjoy Kumawat

Posted on

My Open Source PR Added 285 Lines of Docs. A Reviewer Told Me to Delete Most of Them. They Were Right.

I went looking for a small, well-scoped contribution to nodejs/undici and found one: eight built-in interceptors (dump, retry, redirect, decompress, responseError, dns, cache, deduplicate) had zero dedicated API documentation. You could find them mentioned in passing, but there was no single page that explained what each one did, what options it took, or how to compose them. Straightforward gap, straightforward fix: write the docs.

I did the obvious thing. Created docs/docs/api/Interceptors.md, wrote a section per interceptor with signatures and examples, wired it into the sidebar in site.json, opened PR #5446. Felt done.

The review comment that came back wasn't about anything I'd written being wrong. It was about something I hadn't noticed: Dispatcher.md already had a ## Pre-built interceptors section, and it already covered most of the same ground, just less completely and without dedicated pages per interceptor. My new file wasn't filling a gap so much as duplicating one, badly, in two places at once.

Why I didn't see it

I'd grepped for "interceptor" documentation before starting and found scattered one-line mentions, which is what led me to conclude there was no real coverage. What I hadn't done was read the full existing Dispatcher.md section end to end, because it didn't show up as a dedicated "interceptors" page in the docs sidebar, it was buried as a subsection of a much longer file about a different topic. Grep found the keyword. It didn't find the shape of the existing content.

That's a specific, repeatable mistake: searching for whether something exists by keyword instead of by reading the actual structure of what's already there. The keyword search told me interceptors were mentioned. It didn't tell me they were already documented, just in a place I wasn't looking.

What "fixing" it actually meant

My first instinct on getting the comment was to figure out how to keep both sections and cross-link them better. That's the wrong instinct, and it's worth naming why: it's the instinct to preserve your own prior work rather than admit the first draft solved the wrong problem. Cross-linking two overlapping sections doesn't remove the duplication, it just makes the duplication easier to find, which isn't the same thing as fixing it.

The actual fix the reviewer (metcoder95) asked for was blunt: delete the ## Pre-built interceptors section from Dispatcher.md almost entirely, replace it with a short pointer to the new dedicated page, and fix everything that had been linking into the now-removed section.

- ## Pre-built interceptors
-
- Undici ships with the following interceptors:
- ...
- <282 more lines: one subsection per interceptor,
-   duplicated from what Interceptors.md now covers>
+ For the full list of built-in interceptors, their options,
+ and usage examples, see [Interceptors](./Interceptors.md).
Enter fullscreen mode Exit fullscreen mode

Net diff on that commit: -285 lines, +3, plus removing 9 now-dangling internal link references that pointed at anchors inside the section I'd just deleted, and adding one new link from the dispatcher.compose() section pointing at the new page instead. The commit that did this, fbac28a1, is smaller and less interesting to write than the original PR, and it's the commit that actually made the contribution good.

The part that generalizes

Adding documentation feels productive in a way that auditing existing documentation for overlap doesn't. Writing a new page has a visible, positive artifact at the end. Checking whether your new page duplicates an old one has no artifact at all if you do it right, you just don't create the duplicate in the first place, and nobody sees the bug you didn't ship.

That asymmetry is exactly why the mistake is easy to make and easy to repeat. There's no natural checkpoint in "write new docs" that prompts you to go looking for old docs covering the same ground unless you build one in deliberately. Since this PR, the checkpoint I actually use before opening any docs PR is boring but mechanical: grep the existing docs tree for the feature name, not just the term I'm about to write about, and read every file that matches end to end, not just the matching line. grep -rn "interceptor" docs/ would have surfaced Dispatcher.md. Reading past the first hit is the part I skipped.

The reviewer catching this wasn't a case of me writing something factually wrong. Everything in my original Interceptors.md was accurate. It was a case of the PR being more than it should have been, and more documentation isn't automatically better documentation. Two descriptions of the same eight interceptors living in two files means the next person who needs to update interceptor behavior has to remember to update both, and eventually won't, and now the docs are wrong in one of the two places and nobody notices until someone files an issue about it.

Deleting 285 lines I'd written felt like a bigger concession than fixing a bug in them would have. It shouldn't have. The PR that merged is better than the PR I opened, and it's better specifically because most of what I added didn't survive review. If your metric for a good docs contribution is lines added, this PR looks like it went backwards. It didn't. It went from two half-right sources of truth to one complete one, and that's the actual goal, not the line count.

Top comments (0)