|
I work as a tech lead, so I get a lot of leeway in setting process. For small PRs, we use the normal “leave comments, resolve comments” approach. For large PRs, we schedule 30m meetings, where the submitter can explain the changes and answer questions, and record any feedback. This ensures everyone is on the same page with the changes, gives folks a chance to rapidly gather feedback, and helps familiarize devs who do not work in that area with what is going on. If the meeting is insufficient to feel like everyone is on the same page and approves the changes, we schedule another one. These are some of the best meetings we have. They are targeted, educational, and ensure we don’t have long delays waiting for code to go in. Instead of requiring every PR to be small, which has a high cost, I recommend doing this for large/complex projects. One additional thing to note on small PRs: often, they require significant context, which could take hours or even days, to be built up repeatedly. Contrast that with being able to establish context, and then solve several large problems all at once. The latter is more efficient, so if it can be enabled without negative side effects, it is really valuable. I want my team to be productive, and I want to empower them to improve the codebase whenever they see an opportunity, even if it is not related to their immediate task. |
As you say it's much easier to schedule a 30 minute meeting, then we can - with context - resolve any immediate nitpicks you have, but we can also structure bigger things.
'Would this block a release?'
'Can we just get this done in the PR and merge it'
'Ok, so when it's done... what is the most important thing that we need to document?'
Where the fact that even after it's merged, it's going to sit in the repo for a while until we decide to hit the 'release' button', this lets people defer stuff to work on next and defines a clear line of 'good enough'