top | item 45371545

(no title)

ValleZ | 5 months ago

If you split all the changes for a feature this way not only you hide the way all changes interact with each other but also make the development at least 10x longer because an average approval time is often more than a day.

discuss

order

fidotron|5 months ago

This will be accompanied by the sort of dev manager that thinks a KPI for "number of PRs merged" won't in any way be gamed or backfire.

I don't know what they're doing where you can do code reviews in 5-10 minutes, but in my decades doing this that only works for absolutely trivial changes.

StilesCrisis|5 months ago

The goal here is to make almost all CLs trivial enough that they can be reviewed quickly. You can compose almost any feature out of many small simple changes.

viraptor|5 months ago

> only you hide the way all changes interact with each

Splitting the change does not prevent you from looking at diffs of any combination of those commits. (Including whole pr) You're not losing anything.

> at least 10x longer because an average approval time is often more than a day.

Why do you think it would take longer to review? Got any evidence?

eigencoder|5 months ago

I think approval time would take much longer. The issue is that while actual time spent in review may be shorter, there's a lot of context-switching time costs that increase with the number of PRs submitted.

No one on the team is just sitting there refreshing the list of PRs, ready to pick one up immediately. There's a delay between when the PR is marked as ready and when someone can actually get to it. Everyone is trying to get work done and have some time daily in flow state.

Imagine you have a change; you could do it as one PR that takes 1 hour to review, or 3 small PRs that each take 15 mins to review. The time spent in review may even be shorter for the small PRs, but if each PR has a delay of 1 hour before a reviewer can get to it, then the 3 PRs will take almost 4 hours before they're done, as opposed to just 2 hours for one big PR.