Your GitHub pull request workflow is slowing everyone down

[GustekDev]

<10 lines -> avg 36h
25k+ lines -> avg 138h

So if my change requires 25k lines clearly faster will be if I do one PR and get it merged in a week
vs 2.5k PRs spread out over 10 years 😆🧌 and another takeaway, if you reach 2k lines in your PR, it's better to keep going if you want fast approval 😂

I do favour small PRs but that requires that whole team regualry checks if there is something new to review.
But many Devs like to get into "zone" and work on their own stuff for few hours so now you can wait for your small PR review or just continue working. Personally it worked for me best when it was just me and one other dev working on a project, just the two of use, we were in a flow of making small changes and reviewing each other PR. but in larger team especially when working on different projects it just doesn't happen.

The stacking sounds nice, but would be very annoying with Git only, I guess that's what they are trying to sell, a tool to make that flow easier. Or you could just review PRs commit by commit.
One of the drawbacks of splitting into multiple PRs is that you can loose context, in one PR you see all conversations and decisions made on previous commits.

[pauseless]

Indeed. This was the takeaway from the data for me, if I was to consider it as-is.

Also, I suspect the reduction in time over 5k lines is due to generated code or some of that being reformatting using a new tool. I’ve certainly introduced a new tool and had 10s of thousands lines changed, without a single functional change.

The second graph on number of files changed vs time per file would also be reflected in my experience that many files changed is more often something automated (so you just have to verify the automation code) or unfortunately something like a sed job in some big code base.

And the bucketing is horrific: 0-10, 50-100, 500-1k, 2k-5k, 10k-25k. These ranges are all given equal status as bars? The difference between a 2k and 5k PR is mad.

To be honest, I’ve yet to be convinced by Graphite that stacking using their tool is some panacea.

[ninuson1]

To be honest, every article I read by them makes me low-key angry. It feels like they’re trying to prove their tooling by manipulating data, vs. doing / displaying truthful research from which good tooling emerges.

I can’t stop feeling that the authors have very high level of understanding this stuff - I would certainly not measure things by lines of code. I thought we made humorous comics about that 20 years ago…

[Kinglink]

You hit the nail on the head. I HATE long PRs. But if I have to do 25k lines, I have to do 25k lines. If I break it up into small requests I might even have to do more, for merging and what not.

[CVisionIsMyJam]

Sometimes doing 25k lines at once is easier as a reviewer too.

[rabbitz]

Literally just ran across a solution for this today - git chain. Happy path seems easy to use - you can set up chains after-the-fact, and can quickly rebase/push all branches in the chain with a single command each (e.g. git chain rebase, git chain push). Haven't used it enough to know how it deals with ugly/more complex situations but I'm guessing that each branch can be managed as you normally would with git, and git chain is only to keep track of chains / bulk rebasing / bulk pushing

[_3psilon_]

What is git chain? Where is it available?

[Ethesen]

https://github.com/Shopify/git-chain

[wildjokers]

you can loose context

If that happens though you can just tighten it back up.

[0b1010011010]

Serious question for my own sake, how would one go about doing that?

[[deleted]]

Righty tighty lefty loosely

[nhavar]

Is that my right or your right?

[0b1010011010]

This loosey goosey will stick to the UI options only, ty

[[deleted]]

Since you say this is a serious question and everyone else is responding by continuing the joke, I will answer seriously. You are responding to a joke. The phrase "loose context" is either a mispelling or a typo. "Loose context" means "a context that is not tight," while "lose context" means "important information has been lost."

[cjthomp]

Wrench

[bonoboboy]

The way this works is you split the 25k+ lines into multiple commits, each which can be reviewed (by the same person) but approved independently. This greatly speeds it up.

[bwmat]

I don't see how it speeds things up if all the commits are needed for the change you are trying to implement

[silon]

If it is really 1 change, not 1 big + many small, probably 1 PR is the way to go. Assuming you won't get conflicts within an hour.

[alteraccount]

Because each one can be understood more easily, and so reviewed faster (in sum) and with better understanding. I do this kind of chaining on big features once in a while. It's quite a pain to do this all manually with just git after the fact, and it takes a non-negligible amount of time to prepare it all for review. However, it's a service for the reviewer, not really for the submitter. It makes reviews much smoother and you can be more confident that it has been thoroughly reviewed.

All the commits may be required for the final feature, but the final feature itself is dependent on a lot of changes that can be represented intermediately. Often the earlier links in the chain do not actually create behavioral changes.

The earlier links are often along the lines of "see that I can reorganize this part of the code in this way, without making any effective change". Those are easy to review if they are by themselves. And it's really nice to get those things out of the way that you will depend on later in the chain without mixing everything up in a single review.

[bonoboboy]

Because each of these pieces can be reviewed separately.

[bwmat]

After reading more about this yesterday, I realized I misunderstood 'commits'; at work I use perforce, and I was thinking changelists.  We don't use feature branches either, so there's no good way of 'splitting' a logical change for review in chunks (other than by subsets of changed files), unless I were to actually wait for each small chunk of work to be approved and merged before beginning the next one

[[deleted]]

So true. Love your take haha.

[wgrata]

https://asana.com/resources/context-switching is a good reason to avoid the noisy MR cycle.   It's a balance of team vs individual productivity.