News for hackers. Stuff that matters.
Re:Your GitHub pull request workflow is slowing everyone down by mpweiher on Friday February 23, 2024 @07:27AM (#39479721)
I had some colleagues at a former company who absolutely swore by this.

Being used to trunk-based development, my approach was simply to make small commits. And if you insist on PRs, make small PRs. Although not totally cost free, they are easy enough to create that the overhead is trivial even with very small PR sizes.

I asked them how this improved on just making small PRs, particularly with the need for extra tooling an complexity, and all I got was blank stares.

Reading the blog entry, I don't see anything that goes beyond "just make small PRs. And now here's some tooling that's totally unnecessary if you make small PRs, but you'll have to organize your big PRs as a set of smaller PRs anyway"

What am I missing?

Re:Your GitHub pull request workflow is slowing everyone down by Denvercoder9 on Friday February 23, 2024 @08:14AM (#39480159)
Small PRs are great, but GitHub's workflow sucks if you have dependencies between PRs.

Say I introduce some new abstraction and then convert two components to use it. I would want to create 3 small PRs, one to introduce the abstraction, and then 2 that convert one component each. That's especially useful if the changes to the components need approval from their respective owners. I want to be able to do all this in one go without being blocked by waiting for reviews.

The problem is that GitHub doesn't support this workflow, as it always uses the same branch for the base and the target. I would want to be able to create the first PR against the trunk branch, and then have the subsequent PRs show only the diff against that PR, but merge into trunk (of course you shouldn't be able to merge them until the first PR is merged). Currently, you have to either put everything in a single PR (creating huge, hard to review PRs), or wait until the first PR is merged before you can open the subsequent PRs.

Re:Your GitHub pull request workflow is slowing everyone down by mpweiher on Friday February 23, 2024 @08:30AM (#39480330)
So it's a workaround for "even small PRs take too long to review"?
Re:Your GitHub pull request workflow is slowing everyone down by Denvercoder9 on Friday February 23, 2024 @08:46AM (#39480486)
No, it's a solution for reviews being an asynchronous process. Even if I get a review within an hour or two, I should be able to continue further work in that time.
Re:Your GitHub pull request workflow is slowing everyone down by from-nibly on Friday February 23, 2024 @09:41AM (#39481092)
Isn't it often better to get a single thing all the way through? I know it feels like you are still working on the same feature, but if you want feedback you shouldn't move onto something else while waiting for it. That completely scrambles your flow.

Craming your day with as much "work" as possible isn't faster.

Re:Your GitHub pull request workflow is slowing everyone down by Denvercoder9 on Friday February 23, 2024 @02:32PM (#39485086)
I don't necessarily create the first PR because I want feedback. The design could've been discussed beforehand, and the PR is just to get a second set of eyes on the details, run the CI checks and to fulfill the review requirements. Nothing that comes out of that would have implications for the follow-on PRs, so why shouldn't I continue working on them?
Re:Your GitHub pull request workflow is slowing everyone down by from-nibly on Sunday February 25, 2024 @01:26AM (#39498212)
> is just to get a second set of eyes on the details, run the CI checks and to fulfill the review requirements

AKA feedback

Re:Your GitHub pull request workflow is slowing everyone down by mpweiher on Saturday February 24, 2024 @04:11AM (#39490256)
How is "being an asynchronous process" different from "taking too long", in your opinion?

If reviews are instantaneous, how does that inhibit you from continuing to work?

Re:Your GitHub pull request workflow is slowing everyone down by liamfd on Friday February 23, 2024 @09:52AM (#39481229)
GitHub does support that workflow, if I'm understanding you right.

You can set the target branch on a PR. So pr A targets main, B and C target A. After you merge A, B and C even get automatically retargeted to main (or whatever you merge A into). This handles the "block B and C until A is merged" requirement as well.

Now, you can accidentally merge B and C early by accident into A and mess with the diff, and you need to have your CI properly configured to only check the diff between them and their target (not just main) but it works fine for most use cases in my experience. The other unfortunate thing is the potential for merge conflicts in the stack, of course, if you change A after branching the others off.

Re:Your GitHub pull request workflow is slowing everyone down by Denvercoder9 on Friday February 23, 2024 @02:36PM (#39485119)
The big issue on GitHub is that it doesn't work across forks: I can't create a PR for B that targets A, which both live in my fork, in the parent repository.

The other problem is that without extra tooling it doesn't give a good user experience. It's too easy for B and C to accidentally be merged into A, unless you mark them as WIP, but that requires manually unmarking them when A is ready. There's also no UI to easily navigate across the stack of PRs.

Re:Your GitHub pull request workflow is slowing everyone down by travisby on Friday February 23, 2024 @07:40AM (#39479816)
I used a stacking review system at two other roles (gerrit and phabriactor). At the time I found it annoying and missed the pretty Github UI and the ability to bunch commits together.

Now that I'm using Github full time, I miss the stack. I found stacks allowed me to easily commit for a few problems at once without bogging down a reviewer.

It sounds like you're already a fan of PRs being small, and that probably means _doing one thing_.

What about a spelling mistake in a nearby but unrelated comment? That probably should be a separate PR. In the stack-world, I was happy to make a second commit, create a second review artifact, and be OK with it shipping eventually when the other features of my commit ship (especially if they're close enough to cause a merge conflict!)

Today, I have to: * Stash/commit -m temp my current work * Checkout a new branch, make the fix, push it up, make a PR, etc. * Check back out my old work, potentially rebase on top of it if it's going to cause a merge conflict * get back to work on the real issue * Create a PR for the real issue... and hopefully not have it reviewed until the doc fix is already merged, because it's going to show up as an unrelated change here anyway!

There's also merge-into-other-branches that might help with this workflow, but I haven't found anything graceful yet.

In practice, I avoid the documentation fix to avoid the change in flow, when I'd have been happy to just have it as a different commit in the stack.

--

that covers more generically why I like stacks, I'm not sure graphite would solve my problems after reading the OP though.

Re:Your GitHub pull request workflow is slowing everyone down by Zacharias030 on Friday February 23, 2024 @07:59AM (#39480008)
I have used ghstack for exactly the problem you describe, but I like how graphite managed trees of stacks, which is better than ghstack‘s linear stacks
Re:Your GitHub pull request workflow is slowing everyone down by vasco on Friday February 23, 2024 @07:29AM (#39479735)
Small PRs and more than that, small work chunks are a really good way to get a team going and to be able to be more agile. Estimation is also easier because instead of even T-shirt sizes all you do is "is it small enough?" / "can we finish this task in under 1-2 days?" and if you do that to everything you can get pretty fast (and still offer predictability to the business by measuring throughput).