3 ms·

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. A

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?

vasco 3y ago
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).
travisby 3y ago
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.
- Zacharias030 3y ago
  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
Denvercoder9 3y ago
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.
- mpweiher 3y ago
  So it's a workaround for "even small PRs take too long to review"?
  - Denvercoder9 3y ago
    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.
    - from-nibly 3y ago
      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.
      - Denvercoder9 3y ago
        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?

vasco 3y ago

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).

travisby 3y ago

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.

Zacharias030 3y ago
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

Denvercoder9 3y ago

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.

mpweiher 3y ago
So it's a workaround for "even small PRs take too long to review"?
- Denvercoder9 3y ago
  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.
  - from-nibly 3y ago
    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.
    - Denvercoder9 3y ago
      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?