5 ms·

I really didn't understand what the big deal was with Stacked Commits when I was just working in vanilla github, and it used to cause friction with members of m

I really didn't understand what the big deal was with Stacked Commits when I was just working in vanilla github, and it used to cause friction with members of my team who knew the stacked workflow, but didn't know the Github workflow. After moving to a company that uses stacked commits I will never go back to the pull request workflow. I think there are some famous people saying that Pull Requests aren't Code Reviews, and that becomes obvious once you start using stacked commits for code reviews. This is essentially how cli git is set up to function, and mailing list driven workflows use this concept to have small incremental reviews. I am excited to see this tool, but honestly I have soured so much on github because of the lack of code review, that I would just as soon it didn't try to shoehorn into pull requests, and just did what people do with Phabricator, optionally use GH too host the git repo if you need to, but do everything else in a different tool

notreallyserio 5y ago
I still don't understand the big deal so maybe you could help: what is the difference between stacked commits and pull requests targeting branches in other pull requests? I've used GH for code review so I'm not sure what's missing there, either. Reading this whole comment section leaves me a bit confused, like I'm the only one using a hammer to pound in screws when everyone else has moved on to a screwdriver.
- imiric 5y ago
  GH will also automatically retarget the dependent PRs to the `main` branch after the dependency PR is merged. I usually just make a note in each stacked PR, "Depends on #N", mostly so that the review and merge sequence is followed. But even this would be clear on its own by the PR number and target branch. I agree that the PR code review process leaves a lot to be desired, but this particular workflow doesn't seem like a big problem. Though I've never used Phabricator and similar tooling, so there must be something we're missing.
  - howinteresting 5y ago
    Only for branches in the main repo, not those in forks.
- dkhenry 5y ago
  The big win for me is when someone reviews my code I can easily rebase the entire stack and push it back up for review, and thats all done with `git rebase -i HEAD~n`. That means when someone is actually reviewing my code its not uncommon for me to change something somewhere near the base of the stack and push up changes to the entire stack. You in theory can do that with branch based workflows but its a pain. As I understand this is what graphite is automating. Once you can easily do that, it naturally leads to being able to do review per commit which it makes it so easy to unblock your work because your not fighting against git, and you aren't worried about getting immediate reviews because I know if any changes are requested its trivial to make those changes, and I still have branches, so if I do have unrelated work I can still use branches to isolate them. The last thing is another tool feature, when I rebase and push up changes I have to give a comment on that new commit so reviewers can see the changes that caused the new revision. Honestly I was a heavy user of Github and GitLab having instituted one of them at three different companies, and built out CI pipelines and development procedures for multiple large engineering teams, and I now feel like I was using a hammer to pound in screws and I will never go back.
  - cerved 5y ago
    Could you elaborate what you mean with Stack? I'm confused. Is it some kind of logical grouping of commits that aren't branches?
    - dkhenry 5y ago
      Correct it is just the group of sequential commits. You can keep multiple stacks on a branch, but the big idea is the branch is not the unit of review, each commit is the unit of review
      - cerved 5y ago
        I'm curious, how is this achieved? The way I logically group commits is by stacking branches which are merged without fast forward. Works well with interactive rebase.
  - 5y ago
- howinteresting 5y ago
  It's equivalent but horrible to actually use. Try managing a stack of 40 that way.
  - notreallyserio 5y ago
    That sounds miserable no matter the tooling tbh!
    - howinteresting 5y ago
      It's actually fantastic with the right tooling. Keep in mind that the Linux kernel works this way too, with long series of patches.
      - notreallyserio 5y ago
        I figure I can probably keep two or three problems in my head at once before I start making mistakes, and my branches stay in my head until they're merged or abandoned. 40? It'd be impossible. Maybe I just have a weird brain though (or am just not a good engineer, based on what I've read about stacks elsewhere since reading this post.)
        dkhenry 5y ago
        You wouldn't do 40 completely unrelated things, they would be 40 aspects of a single thing. I would think 40 is getting a bit big as I have trouble getting stacks of more then 15 past a reasonable review, but the good news is ( at least with some tooling ) you can commit partial stacks, so I can let my team review the first 10, while I am working on 20-30 and everything still just works ( even after I merge the first 10 into the main branch )
        howinteresting 5y ago
        The stack of 40 is usually quite related and often would be a 2-3 PRs in GitHub -- it also lets you perform pipelining where you can add new commits to the end of the stack at the same time as merging commits earlier in the stack.

notreallyserio 5y ago

I still don't understand the big deal so maybe you could help: what is the difference between stacked commits and pull requests targeting branches in other pull requests? I've used GH for code review so I'm not sure what's missing there, either. Reading this whole comment section leaves me a bit confused, like I'm the only one using a hammer to pound in screws when everyone else has moved on to a screwdriver.

imiric 5y ago
GH will also automatically retarget the dependent PRs to the `main` branch after the dependency PR is merged. I usually just make a note in each stacked PR, "Depends on #N", mostly so that the review and merge sequence is followed. But even this would be clear on its own by the PR number and target branch. I agree that the PR code review process leaves a lot to be desired, but this particular workflow doesn't seem like a big problem. Though I've never used Phabricator and similar tooling, so there must be something we're missing.
- howinteresting 5y ago
  Only for branches in the main repo, not those in forks.
dkhenry 5y ago
The big win for me is when someone reviews my code I can easily rebase the entire stack and push it back up for review, and thats all done with `git rebase -i HEAD~n`. That means when someone is actually reviewing my code its not uncommon for me to change something somewhere near the base of the stack and push up changes to the entire stack. You in theory can do that with branch based workflows but its a pain. As I understand this is what graphite is automating. Once you can easily do that, it naturally leads to being able to do review per commit which it makes it so easy to unblock your work because your not fighting against git, and you aren't worried about getting immediate reviews because I know if any changes are requested its trivial to make those changes, and I still have branches, so if I do have unrelated work I can still use branches to isolate them. The last thing is another tool feature, when I rebase and push up changes I have to give a comment on that new commit so reviewers can see the changes that caused the new revision. Honestly I was a heavy user of Github and GitLab having instituted one of them at three different companies, and built out CI pipelines and development procedures for multiple large engineering teams, and I now feel like I was using a hammer to pound in screws and I will never go back.
- cerved 5y ago
  Could you elaborate what you mean with Stack? I'm confused. Is it some kind of logical grouping of commits that aren't branches?
  - dkhenry 5y ago
    Correct it is just the group of sequential commits. You can keep multiple stacks on a branch, but the big idea is the branch is not the unit of review, each commit is the unit of review
    - cerved 5y ago
      I'm curious, how is this achieved? The way I logically group commits is by stacking branches which are merged without fast forward. Works well with interactive rebase.
- 5y ago
howinteresting 5y ago
It's equivalent but horrible to actually use. Try managing a stack of 40 that way.
- notreallyserio 5y ago
  That sounds miserable no matter the tooling tbh!
  - howinteresting 5y ago
    It's actually fantastic with the right tooling. Keep in mind that the Linux kernel works this way too, with long series of patches.
    - notreallyserio 5y ago
      I figure I can probably keep two or three problems in my head at once before I start making mistakes, and my branches stay in my head until they're merged or abandoned. 40? It'd be impossible. Maybe I just have a weird brain though (or am just not a good engineer, based on what I've read about stacks elsewhere since reading this post.)
      - dkhenry 5y ago
        You wouldn't do 40 completely unrelated things, they would be 40 aspects of a single thing. I would think 40 is getting a bit big as I have trouble getting stacks of more then 15 past a reasonable review, but the good news is ( at least with some tooling ) you can commit partial stacks, so I can let my team review the first 10, while I am working on 20-30 and everything still just works ( even after I merge the first 10 into the main branch )
      - howinteresting 5y ago
        The stack of 40 is usually quite related and often would be a 2-3 PRs in GitHub -- it also lets you perform pipelining where you can add new commits to the end of the stack at the same time as merging commits earlier in the stack.