Hacker News Reader

Comment by rbren | Hacker News Reader

When we first started OpenHands (fka OpenDevin) [1], AI-generated PRs would get opened with OpenHands as the PR creator/owner. This created two serious problems:

* First, the person who triggered the AI could approve and merge the PR. No second set of human eyes needed. Essentially bypassed the code review process

* Second, the PR had no clear owner. Many of them would just languish with no one advocating for them to get merged. Worse, if one did get merged and caused problems, there was no one you could hold responsible.

We quickly switched strategies--every PR is owned by a human being. You can still see which _commits_ were done by OpenHands, but your face is on the PR, so you're responsible for it.

[1] https://github.com/All-Hands-AI/OpenHands

View on HN

> Worse, if one did get merged and caused problems, there was no one you could hold responsible.

Clearly the merger and approvers are responsible

I want to believe that can work. There's a neat idealism to the final approver being responsible for any bug fixes. It's encourages knowledge sharing prevent knowledge silos; the approver needs to be well versed enough to fix bugs.

But the reality is that it also risks pushing away those who go out their way to conduct more reviews.

Every team I've been on has had a mix of developers, some of whom are responsive to doing code reviews and will pick them up and process them quickly. There are also those developers who drag their feet, only pick up reviews when they're the only one who can review that area of the codebase, and take their time.

The former developers are displaying the habits you'd like to encourage more widely, and yet they find themselves "punished" by getting even more reviews, as people learn to assign things to them when they want a PR handled effectively.

I fear that compounding that by layering on another obligation to the star team members would further push them out.

> Every team I've been on has had a mix of developers, some of whom are responsive to doing code reviews and will pick them up and process them quickly.

Rubber-stamping is not really "reviewing" though.

And leaving a comment "I don't have enough info to review this properly" is a valid review as well. It signals that somebody else needs to pick it up.

> I fear that compounding that by layering on another obligation to the star team members would further push them out.

I get it, but I don't see an alternative.

1. the company culture must value reviews as work, probably even more important than coding

2. the reviewers must be allowed to respond with "I don't feel comfortable reviewing this because I don't have enough context"

Solvable with a review SLO and a round robin auto assigner for reviews.

If reviewing and merging your PR puts me on the hook for fixing anything that breaks, I just won’t review your PR. If I had time to write it myself I wouldn’t need your PR in the first place.

Surely a well-functioning team is high trust enough that the reviewer should shads some of the blame? Of course, the majority might go to the author.

Reply on news.ycombinator.com