DEV Community

Cover image for CODEOWNERS notifies the wrong people
Vivek Kumar
Vivek Kumar

Posted on

CODEOWNERS notifies the wrong people

Someone changes a shared helper. Tightens a return type, nothing dramatic. The diff is clean, tests pass, and the person who owns that directory approves it in about four minutes.

Then four call sites in a completely different module stop working, and the engineer who owns those had no idea the PR existed.

This is not a review process failure. Everyone did their job. CODEOWNERS did exactly what it says on the tin, and it still routed the review to the wrong person.

Authorship and blast radius are different questions

CODEOWNERS is path-based. You change src/shared/format.ts, GitHub checks who owns src/shared/, and asks them to review. That's a permissions model. It answers who is allowed to approve changes here?

Useful question. Just not the one that keeps you up at night.

The risk in a PR usually isn't in the files you touched. It's in the files that depend on them. Those two point in opposite directions through the codebase: ownership points backward at who wrote things, and blast radius points forward at who consumes them. CODEOWNERS only knows how to look backward.

So the reviewer who could have caught it is the one nobody thought to ask.

What you'd need to build instead

Three pieces, and none of them are exotic.

You need to know what changed at the symbol level, not the file level. "This file was modified" is useless. "This exported function's signature changed" is something you can act on. That's a diff parse.

Then you need the consumers. Who calls this thing? The rigorous answer is a type-aware call graph. The answer I actually shipped is ripgrep, because it works on any language, needs no build step, and doesn't care whether your repo currently compiles. It's a worse algorithm that runs everywhere, which on balance I'll take.

Then you map those files back to people. Explicit config first if the repo has one, git blame as fallback, and GitHub's user search to turn a commit email into a handle.

One thing I didn't expect to matter as much as it does: contract files. openapi.yaml, swagger.yaml, .proto, schema.prisma, routes.ts, migrations. These don't export symbols you can grep for callers of, so the whole symbol-extraction approach sails right past them, and they're exactly the files where one careless edit breaks four services. Each one needs its own rule about what a consumer even looks like. For Prisma that's anything importing @prisma/client. For migrations there's no reliable consumer search at all, so the best you can do is flag it loudly and move on.

The objection you're already forming

Yes, grep-based caller detection produces false positives. A symbol called format will match half your codebase. I set a minimum symbol length and a file cap and it's still a heuristic, and it always will be.

Which is why the default mode posts a comment and does nothing else. No reviewers requested, no merges blocked. Read it for a few weeks, see whether it's pointing at anything real, and only then turn on the part that requests reviews.

I'm fairly convinced that's the only way a tool like this survives contact with a real team. Anything that blocks a merge on a guess gets ripped out by the second sprint.

The thing itself

Ripple is a GitHub Action that does all of the above. Apache 2.0, and there's a PR with a real report on it if you'd rather see output than read about it.

If you want to try it on your own repo without anyone noticing:

- uses: vivek5071/ripple@v1
  with:
    github-token: ${{ secrets.GITHUB_TOKEN }}
    dry-run: true
Enter fullscreen mode Exit fullscreen mode

That runs the whole analysis and posts nothing. The report goes to the Actions log.

It's not a CODEOWNERS replacement. Different question, different answer, run both.

Mostly I want to know where the heuristic falls apart on repos bigger than mine, so if you try it and it produces nonsense, I'd like to hear about it.

Top comments (0)