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.<p>We already have lints, type-checking and some specs in place. But no manual reviews, yet.<p>While we were in-office we could do simple over the shoulder code reviews and exchange valuable feedback fast without any official process.<p>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?
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.
"Over the shoulder" reviews sound like they are halfway between code reviews and pair programming.<p>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.<p>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).<p>I found screen sharing to work well for pair programming, so I don't see why it wouldn't work for live reviews either.
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.<p>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.
Call with screenshare when there's some issue to ask about/review?<p>On the whole I prefer it to 'over the shoulder' I think - 'no that bit, where I'm pointing but you can't see, gah' is annoying, but I get my own screen to look at.<p>(Even better might be some kind of remote editor session, so I could connect from whatever editor I like to use, on my screen at my resolution with my colours, and see your cursor move around while you talk. No idea how you'd do that in a cross-editor sort of way, though I suppose you could support major ones in a way that enabled them to be used with each other, e.g. I view in vim while partner moves cursor in VSCode or whatever.)
>I have some positional power, so I can experiment a little with not-so-much resistance.<p>I've found power from being competent to be more effective.<p>>While we were in-office we could do simple over the shoulder code reviews and exchange valuable feedback fast without any official process.<p>What is this trying to solve that regular code review doesn't? Why does it need to be over the shoulder?<p>>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?<p>Require code review for any non-trivial change?
What was the point of "over the shoulder?" Did you do that to avoid blame? Or avoid someone outside the team seeing code review comments? This smacks of cultural fear.<p>Can you pop open an un-audited video session and have the reviewer drive while they inspect and talk about what they see?
VS Code Liveshare sessions allow you to join your colleague's VS Code to do an over the shoulder code review. Really recommend it for pairing and for some code reviews.<p>Pycharm also has a similar service if you use Pycharm.
If you are going to do a proper review and wish to run stuff on your own you always risk problems with dependencies and environment itself.<p>You could solve this with devcontainer.json and using tools which support this such as Codespaces or Codeanywhere. In addition in Codeanywhere you can share the screen from inside the IDE, have collaborative terminal or code sharing with location following.
> While we were in-office we could do simple over the shoulder code reviews and exchange valuable feedback fast without any official process.<p>I've found out that while over the shoulder code review is good, pair programming (in remote settings or not) is even better. Then you can exchange feedback while coding, which can be more effective than a code review. There are multiple tools like Codeanywhere or Code-With-Me (IntelliJ IDEA), Liveshare in VSCode, etc. that facilitate remote pair programming.<p>I am heading a couple of teams who are also now fully remote and what I usually see is that the engineers make fairly small and self-contained PRs. Those *generally* don't call for a over-the-shoulder code review. Keep in mind that we simply never merge code without a code-review regardless of how small the change is, so most engineers in our organization use small, easy-to-digest PRs in order to make it easier for their colleagues to reason about the change.<p>If you find that a PR has attracted a lot of comments and back-and-forth discussion, it's good to encourage the devs to jump on a call or go to a meeting room to try to clear it up synchronously by talking it out. One thing to always consider is that in a fully remote setting you probably don't want to have a lot of those talks going on all the time as they can be fairly disruptive and time-consuming. Having lots of those meetings can be a sign of a couple of things:<p>- maybe the PRs are too big to digest and create lots of questions<p>- or maybe team members were not on the same page about the implementation BEFORE coding<p>- or (rarely) your teams don't have an agreement on consistent coding practices. For example: back-and-forth about tabs vs spaces, linting, or ways to do repeatable tasks like logging or class/variable naming, etc.<p>TLDR;<p>- Always requiring a code review for every PR is a good thing. With time your team will work well and everybody's gonna be knowledgeable what others are working<p>- Keep PRs/code-changes small. That reduces the need for over the shoulder-code-reviews which most of the time can be substituted with a simple approve on the PR<p>- Over-the-shoulder code reviews are good but pair-programming sessions (remote or not) are even better<p>- If you do have to jump on a call about a code change, that shows that maybe some of the above things didn't work out. Still go for it as soon as you find out async comments on the PR don't work
Generally, it’s been useful for my teams to have open discussions about establishing coding standards amongst themselves. We’ve also made code reviews mandatory as part of pull requests although it’s not as effective for smaller teams that want to be more agile.
ssh tunneled VNC clients.<p>The best programming I’ve done is remote with the other person staring at the same screen as me. We also had the same size screen so there was no scaling and such.<p>Makes switching between drivers seamless. Unfortunately my current team uses screen share on video chat apps. It’s the worst.