My friend and I have a game where we talk about what we'd do if we were rich. Not rich like 'paid off the mortgage' rich. Rich like a man who owns a submarine he's never been inside. Rich like a man whose third wife has a skincare
The global GDP lost annually to senior engineers staring at a four-line PR waiting for someone — anyone — to type ‘LGTM’ could fund a moon mission.
The very next line is true though and entertaining. Has nothing to do with LLM’s though. This is a truly vexing process issue. The number of teams that do this rather than just pushing to trunk and relying on proper CI is way too high.
I find it frustrating when I get a review request that has nothing complex. Like, “What do you want me to review here? Spelling?” They really just want to do the process. Open a patch, waste my time clicking approve. Then, immediately merge. Just push to trunk, chicken.
Sometimes things look simple but have complex knock-on effects, and it takes an expert to notice that there’s anything that might be a problem. At least none times or of ten, there’s no problem and it looks like someone’s wasting their time, but it occasionally saves a much bigger headache later.
No doubt, but not always, and sometimes the person making the patch already is an expert and I trust them. There are times when a pull request is not necessary. Just apply the patch. Especially if it is the same patch applied across many repositories.
Process for a good reason? Great. Process for the sake of rote process? Not as great.
Well, that’s a bit of a “How long is a piece of string” question. I’m not saying there is a clear delineation, just that most programmers could see that changing one line with little practical impact is wholly different than a patch with many restructured components. Where between those two a team draws the line is a fair question but there is a line, and assuming all changes, regardless of impact, are on one side of it does waste time.
Sure, maybe 2 minutes. But not only does each 2 minute interruption actually cause a full break in current work; context switching is notoriously costly for programmers, but they also add up. If I get many of these in one day, that just isn’t productive. Imagine a teammate decides we’re going to upgrade a library dependency for a patch version. We validate this. Then, we simply need to change this version id in each of 50 services. Opening a PR for each of these is craziness.
Sure, maybe 2 minutes. But not only does each 2 minute interruption actually cause a full break in current work; context switching is notoriously costly for programmers
small Code Reviews should not warrant an interruption of your work, because not reviewing them should not block anyone. they should be done when someone is actually idling or at a planned time. most of the time i’ll review small changes directly after a daily if someone mentions a small open pr before starting with my actual work, i definitely prefer this to people just merging things and losing the chance to ask questions.
^w
The very next line is true though and entertaining. Has nothing to do with LLM’s though. This is a truly vexing process issue. The number of teams that do this rather than just pushing to trunk and relying on proper CI is way too high.
I find it frustrating when I get a review request that has nothing complex. Like, “What do you want me to review here? Spelling?” They really just want to do the process. Open a patch, waste my time clicking approve. Then, immediately merge. Just push to trunk, chicken.
Sometimes things look simple but have complex knock-on effects, and it takes an expert to notice that there’s anything that might be a problem. At least none times or of ten, there’s no problem and it looks like someone’s wasting their time, but it occasionally saves a much bigger headache later.
No doubt, but not always, and sometimes the person making the patch already is an expert and I trust them. There are times when a pull request is not necessary. Just apply the patch. Especially if it is the same patch applied across many repositories.
Process for a good reason? Great. Process for the sake of rote process? Not as great.
when is a change complex enough to warant a review and how do you ensure that no complex change is merged without a review?
Sounds the real issue is that work is poorly defined if it comes to a standstill if no one is available for a 2 minute review.
those 2 minutes reviews are noise at the end of the day, if you ask me, they don’t have a noticeable impact on the reviewers daily performance.
Well, that’s a bit of a “How long is a piece of string” question. I’m not saying there is a clear delineation, just that most programmers could see that changing one line with little practical impact is wholly different than a patch with many restructured components. Where between those two a team draws the line is a fair question but there is a line, and assuming all changes, regardless of impact, are on one side of it does waste time.
Sure, maybe 2 minutes. But not only does each 2 minute interruption actually cause a full break in current work; context switching is notoriously costly for programmers, but they also add up. If I get many of these in one day, that just isn’t productive. Imagine a teammate decides we’re going to upgrade a library dependency for a patch version. We validate this. Then, we simply need to change this version id in each of 50 services. Opening a PR for each of these is craziness.
small Code Reviews should not warrant an interruption of your work, because not reviewing them should not block anyone. they should be done when someone is actually idling or at a planned time. most of the time i’ll review small changes directly after a daily if someone mentions a small open pr before starting with my actual work, i definitely prefer this to people just merging things and losing the chance to ask questions.