News for hackers. Stuff that matters.
Re:Improving code review time at Meta by kweingar on Thursday November 17, 2022 @05:47PM (#33647043)
I work at a company with a similar code review culture.

I have about 2 blocks of meetings per day on average. I usually check my assigned code reviews in the morning and when I come back from the meetings. Once I’m done with those then I move on to my own work.

If I have too much code review to the point that I can’t get my own work done, I take my name off the reviewer list and it’s reassigned.

On most days I’m still able to get a good 2-3 hours of sustained focused work on my own project.

Re:Improving code review time at Meta by zadjii on Thursday November 17, 2022 @06:08PM (#33647272)
> I take my name off the reviewer list and it’s reassigned.

How big is your team? We've got 4 developers on our team (and one lead), and require two signoffs for code changes. There's just not enough of us for something like that to work... right?

Re:Improving code review time at Meta by aftbit on Thursday November 17, 2022 @06:37PM (#33647581)
That doesn't sound like the best ratio to me, but I don't know what you are running or how good your deploy process is at catching issues. With a 4 person team, you are probably better off discussing risky/bigger changes as a whole team, and allowing 1 review for smaller, less risky changes with well defined rollback processes.
Re:Improving code review time at Meta by carlmr on Thursday November 17, 2022 @08:59PM (#33648846)
Also with a team so small you can't afford wasting it on meetings.

You might need focus, i.e. non-meeting days, 3 days a week.

Re:Improving code review time at Meta by flamebreath447 on Thursday November 17, 2022 @10:49PM (#33649936)
I work on similar sized teams, I introduce my PRs as stacked so they are extremely consumeable in very small bite-sized code changes and build on the last one reviewed.
Re:Improving code review time at Meta by RussianCow on Friday November 18, 2022 @01:46AM (#33651206)
Do you use any special tools for this? I've heard of people "stacking" diffs in this way, but it seems like it would be clunky to review on GitHub.
Re:Improving code review time at Meta by return_to_monke on Friday November 18, 2022 @03:29AM (#33651903)
This is one of the features of (yes), Meta's sapling VCS. https://sapling-scm.com/docs/introduction/getting-started/
Re:Improving code review time at Meta by Dunedan on Saturday November 19, 2022 @08:23AM (#33669809)
If you work directly in the source repository that works actually really well on Github (if everybody works on its own fork instead, you're unfortunately out of luck). When opening a pull request, you just select the branch the pull request should be based on to be the previous branch in your stack. If one of those pull requests gets merged, Github will automatically rebase the stacked on onto the base branch. This way every pull requests contains exactly the changes you want it to contain, while you're still being able to stack changes by opening multiple pull requests, with each one being based on the branch of the previous one.
Re:Improving code review time at Meta by kevingoslar on Saturday November 19, 2022 @08:26PM (#33677091)
Re:Improving code review time at Meta by newaccount74 on Friday November 18, 2022 @04:40AM (#33652440)
So every change needs to be seen by 3 out of 5 people (assuming the lead also does code review)? That sounds a bit inefficient.
Re:Improving code review time at Meta by HereBeBeasties on Friday November 18, 2022 @03:50AM (#33652047)
I think your main issue is having "two blocks of meetings" a day as a developer. That sounds absolutely nuts to me. Even two meetings a day is likely too much for a dev, let alone two blocks of meetings! I'd start by reducing that.
Re:Improving code review time at Meta by radiKal07 on Friday November 18, 2022 @04:01AM (#33652135)
> I have about 2 blocks of meetings per day on average

Are you sure you are a developer?