I'm searching for resources about code review processes. Feel free to reply with your experiences too. I'm not interested in technical matters right now like how to automate parts of code review or your Opinion about the right size of a pull request. I'm asking about the social process of deciding when to merge. How do you ensure quality and keep development moving ahead without getting dragged down by disagreements? Boosts appreciated.

@be I like how Hintjens handled it and how it's formalized in C4. When in doubt, merge. If it passes any automatic gates set up and isn't totally bonkers, in it goes.

Potentially this might lead to edit wars, but long-lived branches are a curse, and the threshold for making a PR is so much higher than leaving a comment expecting someone else to make the change.
At work our tests aren't quite good enough yet to be this liberal and merge this fast, but as the senior on the team I will more often leave an Approve with comments than I will leave a Changes Requested that blocks the process. I may also leave a comment to please get this merged as is, but please follow up with a PR that improves XYZ.

@clacke I tried this approach in one project, but we ended up in a situation "delivery" teams were developing mediocre code and a few folks from an infrastructure team were chasing new features by cleaning up and writing tests.

How do you make sure that XYZ improvement gets done?

Β· Β· Web Β· 1 Β· 0 Β· 1
@dima This is a risk, and we've been there. To counter it, we are slowing down on feature development and allowing ourselves to catch up on refactoring. We're also being slightly stricter on the reviews. It's a delicate balance.

I'm still convinced that this way is better than leaving PRs to rot, which is highly discouraging.

@clacke I agree, nobody likes maintaining an old PR.

> we are slowing down on feature development and allowing ourselves to catch up on refactoring.

Are developers ultimately in charge for the commitment and responsible for the delivery? How does product owner and other parties react when commitment is not aligned with their plans?

@dima We have agreed with the PO to slow down and they have understood and respected why we want to do it.

@dima @clacke The last straw for me was people continuing to demand I fix trivial issues on 3 year old +/- 10,000 branch rewriting an entire subsystem meanwhile they didn't even start reviewing for 2.5 years and expected me to resolve merge conflicts indefinitely.

@dima @clacke In this specific case, breaking up the branch was not an option because it required breaking a major feature to clear the path for doing the rewrite.

Sign in to participate in the conversation
3dots.lv Mastodon Instance

The social network of the future: No ads, no corporate surveillance, ethical design, and decentralization! Own your data with Mastodon!