Ask HN: Over the shoulder Code-Review in remote settings

#1

Post by aman-pro » Fri, Oct 21, 2022, 7:09 AM UTC

I am trying to set up some good code review practices in my team. I have some positional power, so I can experiment a little with not-so-much resistance.

We already have lints, type-checking and some specs in place. But no manual reviews, yet.

While we were in-office we could do simple over the shoulder code reviews and exchange valuable feedback fast without any official process.

Since now my team is remote (although in the same timezone), what changes can we start making in our team to introduce code review practices to make the code quality better and avoid the common mistakes?

Re: Ask HN: Over the shoulder Code-Review in remote settings

#2

Post by necovek » Fri, Oct 21, 2022, 7:42 AM UTC

"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.

Re: Ask HN: Over the shoulder Code-Review in remote settings

#3

Post by aman-pro » Fri, Oct 21, 2022, 8:11 AM UTC

necovek wrote: post #2

"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 t…

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.

Re: Ask HN: Over the shoulder Code-Review in remote settings

#4

Post by davidajackson » Fri, Oct 21, 2022, 3:10 PM UTC

What size is the company? For a big company, just prevent merge to main unless approved by another team member. Set up pairings, like JuniorSenior reviews. The junior reviewer will learn a low from the senior coding style.

For very small companies, ask yourself if you have time for code review. If you do, great -- but sometimes you are all just burning the midnight oil in those 1-5 person companies just to KTLO. Maybe do with less frequency if company is tiny and doesn't have PMF.

Re: Ask HN: Over the shoulder Code-Review in remote settings

#5

Post by matthewwolfe » Sun, Oct 23, 2022, 2:24 AM UTC

I’ve found that pull request size solves a lot of the issues that people have with code reviews and quality. When people see a very large pull request, there is a tendency to skim and then slap on an Approval. Keeping pull requests small typically leads to a more thorough review because it’s much easier to parse the changes and build a mental model. This usually leads to better feedback. This also helps prevent less experienced devs from going crazy down the rabbit hole and making a huge code change. Small and steady is best, and fostering a culture where people are often asking each other questions and collaborating is key.

Re: Ask HN: Over the shoulder Code-Review in remote settings

#6

Post by sooyoo » Sun, Oct 23, 2022, 7:42 AM UTC

davidajackson wrote: post #4

What size is the company? For a big company, just prevent merge to main unless approved by another team member. Set up pairings, like Junior Senior reviews. The junior reviewer will learn a low from the senior coding style. For very small companies, ask yourself if you have time for code review. If you do, great -- but sometimes you are all just burning the midnight oil in those 1-5 person companies just to KTLO. May…

I find the distinction you make interesting. To me, code review is an essential tool for code quality. Not just to avoid merging problematic code but even spreading knowledge of design and approaches and unifying style. Skipping it necessarily reduces code quality.

In other words, why would code quality matter more in a larger company, or why would programmers in smaller companies be able to somehow magically produce relatively higher quality so that reviews are not crucial for them?

Re: Ask HN: Over the shoulder Code-Review in remote settings

#7

Post by jackblemming » Sun, Oct 23, 2022, 7:51 AM UTC

>I have some positional power, so I can experiment a little with not-so-much resistance.

I've found power from being competent to be more effective.

>While we were in-office we could do simple over the shoulder code reviews and exchange valuable feedback fast without any official process.

What is this trying to solve that regular code review doesn't? Why does it need to be over the shoulder?

>what changes can we start making in our team to introduce code review practices to make the code quality better and avoid the common mistakes?

Require code review for any non-trivial change?

Re: Ask HN: Over the shoulder Code-Review in remote settings

#8

Post by jackblemming » Sun, Oct 23, 2022, 7:53 AM UTC

aman-pro wrote: post #3

necovek wrote: post #2

"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 t…

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 don…

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.

Re: Ask HN: Over the shoulder Code-Review in remote settings

#9

Post by dottedmag » Sun, Oct 23, 2022, 8:01 AM UTC

aman-pro wrote: post #3

necovek wrote: post #2

"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 t…

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 don…

> 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".

Re: Ask HN: Over the shoulder Code-Review in remote settings

#10

Post by jlund-molfese » Sun, Oct 23, 2022, 8:04 AM UTC

sooyoo wrote: post #6

davidajackson wrote: post #4

What size is the company? For a big company, just prevent merge to main unless approved by another team member. Set up pairings, like Junior Senior reviews. The junior reviewer will learn a low from the senior coding style. For very small companies, ask yourself if you have time for code review. If you do, great -- but sometimes you are all just burning the midnight oil in those 1-5 person companies just to KTLO. May…

I find the distinction you make interesting. To me, code review is an essential tool for code quality. Not just to avoid merging problematic code but even spreading knowledge of design and approaches and unifying style. Skipping it necessarily reduces code quality. In other words, why would code quality matter more in a larger company, or why would programmers in smaller companies be able to somehow magically produce…

But that’s what GP is saying! It took me time to adapt to that idea too, going from a fortune 50 company to a startup.

Code quality is less important at a small company or startup, where you don’t know if the code you write (or the company itself) will even be around in a few years. So building a buggy feature quickly is usually better than a solid, well-tested and reviewed one that takes a lot of time to get out the door.