Hacker News new | ask | show | jobs
by necovek 1332 days ago
"Over the shoulder" reviews sound like they are halfway between code reviews and pair programming.

Code reviews usually mean someone taking an independent look at their own pace, though quick turnaround is always great. You can get classic reviews by simply mandating approvals on pull requests on any system you are using.

You can also get halfway with screen sharing developer's IDE, though if a reviewer doesn't get to easily jump around the codebase, it's not really the same thing (though it's equivalent to what you've been doing in person).

I found screen sharing to work well for pair programming, so I don't see why it wouldn't work for live reviews either.

1 comments

Over the shoulder is often the developer explaining their decisions in the code, instead of the reviewer trying to reverse-engineer it, independently. It's just faster and has less resistance -- not necessarily better.

Problem with remote live reviews is that in a remote environment, it's harder to tell if someone is free or they are doing their own deep work.

Either the developer has to wait for the review to be done asynchronously before the merge... or ping someone to review their code through a screenshare and take away their attention.

The reviewer can ask questions on the pr? You shouldn't need to reverse engineer code to figure out their decisions. If it's really that noteworthy the programmer should leave a comment explaining their decision.
My team posts their PR reviews in our team slack channel. Which bothers the totally async purist in me, but we’ve found it to be a good middle ground between waiting for reviews requested via GitHub email and actually pinging someone.

If something’s super complex, and I don’t feel like it’s the submitter’s fault, or even if I’ve got a PR where I realize the code I’ve written isn’t optimal, and async feedback would be too slow, I’ll usually schedule a meeting on someone’s calendar, giving them at least a day of notice.

But otherwise? If I can’t understand what’s going on without too much difficulty, that’s on the PR submitter to improve their code readability through structuring, naming, comments, or as a last resort, external documentation imo. So I see not having the original submitter involved as a feature. Who knows if they’ll be around when you have questions about the same code in 2 years? Is someone going to document everything that was said in the “over the shoulder” review?

That being said, the work I do isn’t anything cutting-edge. More complex code justifies more involved review practices.

> My team posts their PR reviews in our team slack channel. Which bothers the totally async purist in me, but we’ve found it to be a good middle ground between waiting for reviews requested via GitHub email and actually pinging someone.

Why don’t you install the GitHub Slack application? It will tell you immediately when somebody asks for a review and it has other useful functionality as well.

Oh, I guess some additional context is that by convention we use Slack emoji to indicate whether we approve, comment or request changes. Then again when it’s merged/deployed. Reviews aren’t specifically requested of individuals, usually just the entire team. That way, if it’s a channel everyone is in, other people can see at a glance whether they should still review the PR.
The github slack integration would post a message in a slack channel and then your team can use emojis to react to it just like you do.
Will have to check that out then, thanks!
> Over the shoulder is often the developer explaining their decisions in the code, instead of the reviewer trying to reverse-engineer it, independently.

If the reviewer needs to reverse-engineer the code to understand it without the author explaining it, this is a strong signal that the code is not high quality and needs more work. In this case, “over the shoulder” is actually bypassing something that a pull request would catch. “It’s not clear what this code does” is a perfectly reasonable cause to ask for changes in a pull request.

> Over the shoulder is often the developer explaining their decisions in the code, instead of the reviewer trying to reverse-engineer it, independently. It's just faster and has less resistance -- not necessarily better.

Ouch, this is a great point. So for code that has to live for a long-ish time over-the-shoulder review is definitely inferior.

I have setup review process in a company I work in, and one of the rules is "the best way to answer the reviewer's comment is to change the code or add a comment".