Hacker News new | ask | show | jobs
by steelframe 939 days ago
Where I work it takes several hours to get a PR past all the merge gates. If I have 5 small PRs that all build one on top of the other, it can take me the better part of 2 weeks to get them all merged, counting all the context-switching, tweaking based on comments and relaunching of the merge checks, etc.

The only way developers are able to make any kind of velocity in their work is to babysit their PRs morning, evening, and night. "Time for bed. Let me crack open my laptop and check to see if my PR made it past the merge checks. Oops, $RANDOM_CHECK failed because the server that runs it was borked. Let me kick that off again and hope it works overnight so first thing in the morning before I leave for the office I can check again to see if there's some other random failure I can fix and then re-kick off the merge checks so they can run while I'm commuting into the office."

In spite of having it broken into 12 neat commits, a reviewer demanded that I break one of my PRs into multiple PRs. I don't work outside of standard business hours, so I've been iterating on it for the past 4 weeks now in between meetings, design reviews, interviews, etc. Meanwhile a team of about 4 people have been blocked waiting for the full patchset to fully go in.

If instead it were one big PR with multiple commits as I originally had it, I can have gotten it merged in a few days.

4 comments

I mean this is mostly just a devops issue though no? PR’s used to take a solid week to merge at my company with a 10 hour CI that had a ~50% success rate when the code worked. But we put a bunch of resources toward build times and ci run time and now it mostly works. Yea, you can circumvent a shitty pr system by just making massive pr’s but wouldn’t it be easier to have the devops team fix your broken system?
Depends on what your code does. A lot of my code runs robots. You can't scale horizontally because you can only fit so many systems in a room before you run afoul of production limits, safety requirements or cooling capacity. You can't scale vertically because zoning. You can't run the tests faster because physics.

So, you either run the expensive tests asynchronously and all that entails or you allow CI to take forever.

In that case, would it make more sense to have a small PR that you start with, get it reviewed, then open smaller PRs to your original PR? That way your approach/code can be reviewed while in your larger PR you tweak and fix tests.
Not as a (good) general solution. Some things are just hard to break down into smaller logical units. E.g. a complicated new driver might introduce thousands of lines of changes. Half a driver usually won't compile, but you could break things down into PRs over individual functions (in call order so there aren't compiler warnings). That's a wildly unnecessary amount of effort though. You could write a skeleton driver first too, but that's rote declarations barely worth reviewing even as part of a larger PR.
> devops team

Ah… about that

Basically the top antipattern in the book
Isn't the problem here the flaky and long running PR gates? There's room for nuance to say "small PRs are a north star but large PRs are sometimes OK too"
You must work at a big tech company. Thats utter insanity.
Well okay, yes, if your process is that broken then you have to make do. In shops with a vaguely reasonable path to merge a PR, time for human reviewers to sign off dominates and smaller changes are the expedient path to get changes merged quickly.