13 ms·

I might be missing something, but what I need is not "stacked PR" but a proper UI and interface to manage single commit: - merge some commits independently whe

I might be missing something, but what I need is not "stacked PR" but a proper UI and interface to manage single commit: - merge some commits independently when partial work is ready. - mark some commit as reviewed. - UI to do interactive rebase and and squash and edit individual commits. (I can do that well from the command line, but not when using the GitHub interface, and somehow not everyone from my team is familiar with that) - ability to attach a comment to a specific commit, or to the commit message. - better way to visualize what change over time in each forced push/revision (diff of diff) Git itself already has the concept of commit. Why put this "stacked PR" abstraction on top of it? Or is there a difference I don't see?

tcoff91 6mo ago
It's basically trying to bring the stacked diff workflow pioneered by Phabricator to GitHub. The idea is that it allows you to better handle working on top of stuff that's not merged yet, and makes it easier for reviewers to review pieces of a larger stack of work independently. It's really useful in larger corporate environments. I've used stacked PRs when doing things like upgrading react-native in a monorepo. It required a massive amount of changes, and would be really hard to review as a single pull request. It has to be landed all at once, it's all or nothing. But being able to review it as smaller independent PRs is helpful. Stacking PRs is also useful even when you don't need to merge the entire stack at once.
- js2 6mo ago
  > stacked diff workflow pioneered by Phabricator Ahem, pioneered by gerrit. But actually, I'm almost certain even that wasn't original art. I think gerrit just brought it to git. https://www.gerritcodereview.com/about.html https://www.gerritcodereview.com/about.html
  - sunshowers 6mo ago
    To my knowledge, stacked diffs were first done in the Linux kernel as stacks of patches sent over email. From there they spread to Google and Facebook. (Source: I worked on Facebook's source control team from 2012-2018 and did a lot of work to enable stacked diffs there.)
    - js2 6mo ago
      Right, I was thinking from a web-based UI. The "pull request" term is from git (AFAIK), but git itself was built to accommodate the earlier concept of mailing patches around. (Source: I've been using version control since RCS/SCCS days and contributed here and there to git in its infancy. Also an early user/contributor to Gerrit.)
      - Izkata 6mo ago
        > The "pull request" term is from git (AFAIK) Possibly from github. It got popularized there at least, encouraging forking code, and is why so many people say "pull request" when they mean "merge request".
        js2 6mo ago
        GitHub took the "pull request" terminology from Git. A kernel developer (say) would have a bunch of changes ready in their local Git server and would request a pull from Linus, hence a pull request. There's literally a command for it: https://git-scm.com/docs/git-request-pull https://git-scm.com/docs/git-request-pull The command is so old it's still written in shell: https://github.com/git/git/blob/master/git-request-pull.sh https://github.com/git/git/blob/master/git-request-pull.sh It was first added July 27, 2005: https://github.com/git/git/commit/ab421d2c7886341c246544bc8dea43c66a44d909 https://github.com/git/git/commit/ab421d2c7886341c246544bc8d... https://lore.kernel.org/git/20050726073036.GJ6098@mythryan2.michonline.com/ https://lore.kernel.org/git/20050726073036.GJ6098@mythryan2.... But even then, it simply codified existing terminology. Ah, someone else did the research, so minimally BitKeeper had the "pull" command first and the term "pull request" falls naturally from that: https://rdnlsmith.com/posts/2023/004/pull-request-origins/ https://rdnlsmith.com/posts/2023/004/pull-request-origins/
    - p-e-w 6mo ago
      At some point, a derivative idea becomes so different from the original one that it’s a novel idea in essence. Just like SMS is ultimately a derivative of cuneiform tablets, and yet it isn’t in any meaningful sense.
  - pabs3 6mo ago
    Gerrit was forked from Rietveld. Not sure if Rietveld or Gerrit are better though. https://github.com/rietveld-codereview/rietveld https://github.com/rietveld-codereview/rietveld https://en.wikipedia.org/wiki/Rietveld_(software) https://en.wikipedia.org/wiki/Rietveld_(software) https://codereview.appspot.com/ https://codereview.appspot.com/
    - deleted 6mo ago
      [deleted]
- eptcyka 6mo ago
  What if main/master moves in between reviews?
  - tcoff91 6mo ago
    Rebase the stack onto main.
  - jrochkind1 6mo ago
    Before this feature when you were doing it manually, it was a huge problem. One of the points of this feature, is it automates rebasing the whole stack.
  - mh2266 6mo ago
    you just rebase it? what's the big deal? I don't use Github but I do work at one of the companies that popularized this workflows and it is extremely not a big deal. Pull, rebase, resolve conflicts if necessary, resubmit.
  - adregan 6mo ago
    You head to the farthest branch in the chain, fetch the latest main, and run `git rebase --update-refs main` (I prefer interactive mode myself) and then force push all of the branches from start to the end. 1: https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---update-refs https://git-scm.com/docs/git-rebase#Documentation/git-rebase...
    - eptcyka 6mo ago
      I know what one does manually, but how does stack make decisions here when there’s a conflict? The reason we use a rebase flow is to allow for adjusting commits that are no longer valid.
- jrochkind1 6mo ago
  I'm not in a large corporate environment, but that also means we're not always a well oiled machine, and sometimes i am writing faster than the reviewer can review for a period of time -- and i really need the stacking then too.
  - marktani 6mo ago
    > I'm not in a large corporate environment, but that also means we're not always a well oiled machine haha, I'm sitting in a very crusty corporate environment right now and your comment made me chuckle. I get where you're coming from though, of course!
    - jrochkind1 6mo ago
      Ha, yeah, of course I know too! I should have said it different, I meant other people were talking about needing this feature for very large teams, but I find it's needed for very small teams for perhaps different reasons related to their smallness! I need to stack PR's because I need to keep writing on top of the completed PR(s) but it might be a few days until someone else can get to reviewing because there are only a few of us and we're all doing everything.
- ttoinou 6mo ago
  Even with one developer on a repo this sounds like useful
xixixao 6mo ago
Workflows can vary, but what I like: PR/MR is an "atomic" change (ideally the smallest change that can be landed separately - smallest makes it easier to review, bisect and revert) Individual commits (or what "versions" are in Phabricator) are used for the evolution of the PR/MR to achieve that change. But really I have 2 use cases for the commits: 1. the PR/MR is still too big, so I split it into individual commits (I know they will land together) 2. I keep the history of the evolution of the PR/MR in the commits ("changed foo to bar cause its a better approach")
mikeocool 6mo ago
Constantly rewriting git history with squashes, rebases, manual changes, and force pushes has always seemed like leaving a loaded gun pointed at your foot to me. Especially since you get all of the same advantages with plain old stream on consciousness commits and merges using: git merge --no-ff git log --first-parent git bisect --first-parent
- OJFord 6mo ago
  Until someone merges master into their feature branch rather than rebasing it. (And then that branch later gets merged.)
  - sheept 6mo ago
    This shouldn't be a problem if you stick to commits and merges. --first-parent will skip past commits, including merge commits, in merged branches.
    - OJFord 6mo ago
      Fair – but not if it's not their feature branch but their local master; they pull & merge the remote changes and then push the result.
- OptionOfT 6mo ago
  I agree. PR merges for me are bisect points. That's when changes are introduced. Individual commits don't even always build. And I don't rebase or squash because I need provenance in my job.
- MrJohz 6mo ago
  I find rebases are only a footgun because the standard git cli is so bad at representing them - things like --force being easier to write than --force-with-lease, there being no way to easily absorb quick fixes into existing commits, interdiffs not really being possible without guesswork, rebases halting the entire workflow if they don't succeed, etc. I've switched over pretty much entirely to Jujutsu (or JJ), which is an alternative VCS that can use Git as its backend so it's still compatible with Github and other git repos. My colleagues can all use git, and I can use JJ without them noticing or needing to care. JJ has merges, and I still use them when I merge a set of changes into the main branch once I've finished working on it, but it also makes rebases really simple and eliminates most of the footguns. So while I'm working on my branch, I can iteratively make a change, and then squash it into the commit I'm working on. If I refactor something, I can split the refactor out so it's in a separate commit and therefore easiest to review and test. When I get review feedback, I can squash it directly into the relevant commit rather than create a new commit for it, which means git blame tends to be much more accurate and helpful - the commit I see in the git blame readout is always the commit that did the change I'm interested in, rather than maybe the commit that was fixing some minor review details, or the commit that had some typo in it that was fixed in a later commit after review but that relationship isn't clear any more. And while I'm working on a branch, I still have access to the full history of each commit and how it's changed over time, so I can easily make a change and then undo it, or see how a particular commit has evolved and maybe restore a previous state. It's just that the end result that gets merged doesn't contain all those details once they're no longer relevant.
d0mine 6mo ago
You are describing gerrit. https://www.gerritcodereview.com/ https://www.gerritcodereview.com/
ahmadyan 6mo ago
the best implementation i've worked with was SuperSmartLog (SSL) at Meta, which was open-sourced at interactive smartlog (https://sapling-scm.com/docs/addons/isl/ https://sapling-scm.com/docs/addons/isl/). There are also extension for it in VSCode, etc. Surprisingly it never gained the adoption it deserved.
icy 6mo ago
We’ve got this over on Tangled. :) https://tangled.org https://tangled.org
rtpg 6mo ago
yeah interdiffing and being able to cherrypick in a review just there and getting it done is nice. Perhaps a future iteration of this feature will at least allow us to do something like merge just steps of it if they can be reordered.
LuttelBurchtje 6mo ago
[dead]
trashburger 6mo ago
You want something like Gerrit.
ctdinjeu3 6mo ago
[dead]

tcoff91 6mo ago

It's basically trying to bring the stacked diff workflow pioneered by Phabricator to GitHub. The idea is that it allows you to better handle working on top of stuff that's not merged yet, and makes it easier for reviewers to review pieces of a larger stack of work independently. It's really useful in larger corporate environments. I've used stacked PRs when doing things like upgrading react-native in a monorepo. It required a massive amount of changes, and would be really hard to review as a single pull request. It has to be landed all at once, it's all or nothing. But being able to review it as smaller independent PRs is helpful. Stacking PRs is also useful even when you don't need to merge the entire stack at once.

js2 6mo ago
> stacked diff workflow pioneered by Phabricator Ahem, pioneered by gerrit. But actually, I'm almost certain even that wasn't original art. I think gerrit just brought it to git. https://www.gerritcodereview.com/about.html https://www.gerritcodereview.com/about.html
- sunshowers 6mo ago
  To my knowledge, stacked diffs were first done in the Linux kernel as stacks of patches sent over email. From there they spread to Google and Facebook. (Source: I worked on Facebook's source control team from 2012-2018 and did a lot of work to enable stacked diffs there.)
  - js2 6mo ago
    Right, I was thinking from a web-based UI. The "pull request" term is from git (AFAIK), but git itself was built to accommodate the earlier concept of mailing patches around. (Source: I've been using version control since RCS/SCCS days and contributed here and there to git in its infancy. Also an early user/contributor to Gerrit.)
    - Izkata 6mo ago
      > The "pull request" term is from git (AFAIK) Possibly from github. It got popularized there at least, encouraging forking code, and is why so many people say "pull request" when they mean "merge request".
      - js2 6mo ago
        GitHub took the "pull request" terminology from Git. A kernel developer (say) would have a bunch of changes ready in their local Git server and would request a pull from Linus, hence a pull request. There's literally a command for it: https://git-scm.com/docs/git-request-pull https://git-scm.com/docs/git-request-pull The command is so old it's still written in shell: https://github.com/git/git/blob/master/git-request-pull.sh https://github.com/git/git/blob/master/git-request-pull.sh It was first added July 27, 2005: https://github.com/git/git/commit/ab421d2c7886341c246544bc8dea43c66a44d909 https://github.com/git/git/commit/ab421d2c7886341c246544bc8d... https://lore.kernel.org/git/20050726073036.GJ6098@mythryan2.michonline.com/ https://lore.kernel.org/git/20050726073036.GJ6098@mythryan2.... But even then, it simply codified existing terminology. Ah, someone else did the research, so minimally BitKeeper had the "pull" command first and the term "pull request" falls naturally from that: https://rdnlsmith.com/posts/2023/004/pull-request-origins/ https://rdnlsmith.com/posts/2023/004/pull-request-origins/
  - p-e-w 6mo ago
    At some point, a derivative idea becomes so different from the original one that it’s a novel idea in essence. Just like SMS is ultimately a derivative of cuneiform tablets, and yet it isn’t in any meaningful sense.
- pabs3 6mo ago
  Gerrit was forked from Rietveld. Not sure if Rietveld or Gerrit are better though. https://github.com/rietveld-codereview/rietveld https://github.com/rietveld-codereview/rietveld https://en.wikipedia.org/wiki/Rietveld_(software) https://en.wikipedia.org/wiki/Rietveld_(software) https://codereview.appspot.com/ https://codereview.appspot.com/
  - deleted 6mo ago
    [deleted]
eptcyka 6mo ago
What if main/master moves in between reviews?
- tcoff91 6mo ago
  Rebase the stack onto main.
- jrochkind1 6mo ago
  Before this feature when you were doing it manually, it was a huge problem. One of the points of this feature, is it automates rebasing the whole stack.
- mh2266 6mo ago
  you just rebase it? what's the big deal? I don't use Github but I do work at one of the companies that popularized this workflows and it is extremely not a big deal. Pull, rebase, resolve conflicts if necessary, resubmit.
- adregan 6mo ago
  You head to the farthest branch in the chain, fetch the latest main, and run `git rebase --update-refs main` (I prefer interactive mode myself) and then force push all of the branches from start to the end. 1: https://git-scm.com/docs/git-rebase#Documentation/git-rebase.txt---update-refs https://git-scm.com/docs/git-rebase#Documentation/git-rebase...
  - eptcyka 6mo ago
    I know what one does manually, but how does stack make decisions here when there’s a conflict? The reason we use a rebase flow is to allow for adjusting commits that are no longer valid.
jrochkind1 6mo ago
I'm not in a large corporate environment, but that also means we're not always a well oiled machine, and sometimes i am writing faster than the reviewer can review for a period of time -- and i really need the stacking then too.
- marktani 6mo ago
  > I'm not in a large corporate environment, but that also means we're not always a well oiled machine haha, I'm sitting in a very crusty corporate environment right now and your comment made me chuckle. I get where you're coming from though, of course!
  - jrochkind1 6mo ago
    Ha, yeah, of course I know too! I should have said it different, I meant other people were talking about needing this feature for very large teams, but I find it's needed for very small teams for perhaps different reasons related to their smallness! I need to stack PR's because I need to keep writing on top of the completed PR(s) but it might be a few days until someone else can get to reviewing because there are only a few of us and we're all doing everything.
ttoinou 6mo ago
Even with one developer on a repo this sounds like useful

xixixao 6mo ago

Workflows can vary, but what I like: PR/MR is an "atomic" change (ideally the smallest change that can be landed separately - smallest makes it easier to review, bisect and revert) Individual commits (or what "versions" are in Phabricator) are used for the evolution of the PR/MR to achieve that change. But really I have 2 use cases for the commits: 1. the PR/MR is still too big, so I split it into individual commits (I know they will land together) 2. I keep the history of the evolution of the PR/MR in the commits ("changed foo to bar cause its a better approach")

mikeocool 6mo ago

Constantly rewriting git history with squashes, rebases, manual changes, and force pushes has always seemed like leaving a loaded gun pointed at your foot to me. Especially since you get all of the same advantages with plain old stream on consciousness commits and merges using: git merge --no-ff git log --first-parent git bisect --first-parent

OJFord 6mo ago
Until someone merges master into their feature branch rather than rebasing it. (And then that branch later gets merged.)
- sheept 6mo ago
  This shouldn't be a problem if you stick to commits and merges. --first-parent will skip past commits, including merge commits, in merged branches.
  - OJFord 6mo ago
    Fair – but not if it's not their feature branch but their local master; they pull & merge the remote changes and then push the result.
OptionOfT 6mo ago
I agree. PR merges for me are bisect points. That's when changes are introduced. Individual commits don't even always build. And I don't rebase or squash because I need provenance in my job.
MrJohz 6mo ago
I find rebases are only a footgun because the standard git cli is so bad at representing them - things like --force being easier to write than --force-with-lease, there being no way to easily absorb quick fixes into existing commits, interdiffs not really being possible without guesswork, rebases halting the entire workflow if they don't succeed, etc. I've switched over pretty much entirely to Jujutsu (or JJ), which is an alternative VCS that can use Git as its backend so it's still compatible with Github and other git repos. My colleagues can all use git, and I can use JJ without them noticing or needing to care. JJ has merges, and I still use them when I merge a set of changes into the main branch once I've finished working on it, but it also makes rebases really simple and eliminates most of the footguns. So while I'm working on my branch, I can iteratively make a change, and then squash it into the commit I'm working on. If I refactor something, I can split the refactor out so it's in a separate commit and therefore easiest to review and test. When I get review feedback, I can squash it directly into the relevant commit rather than create a new commit for it, which means git blame tends to be much more accurate and helpful - the commit I see in the git blame readout is always the commit that did the change I'm interested in, rather than maybe the commit that was fixing some minor review details, or the commit that had some typo in it that was fixed in a later commit after review but that relationship isn't clear any more. And while I'm working on a branch, I still have access to the full history of each commit and how it's changed over time, so I can easily make a change and then undo it, or see how a particular commit has evolved and maybe restore a previous state. It's just that the end result that gets merged doesn't contain all those details once they're no longer relevant.

d0mine 6mo ago

You are describing gerrit. https://www.gerritcodereview.com/ https://www.gerritcodereview.com/

ahmadyan 6mo ago

the best implementation i've worked with was SuperSmartLog (SSL) at Meta, which was open-sourced at interactive smartlog (https://sapling-scm.com/docs/addons/isl/ https://sapling-scm.com/docs/addons/isl/). There are also extension for it in VSCode, etc. Surprisingly it never gained the adoption it deserved.

icy 6mo ago

We’ve got this over on Tangled. :) https://tangled.org https://tangled.org

rtpg 6mo ago

yeah interdiffing and being able to cherrypick in a review just there and getting it done is nice. Perhaps a future iteration of this feature will at least allow us to do something like merge just steps of it if they can be reordered.

LuttelBurchtje 6mo ago

[dead]

trashburger 6mo ago

You want something like Gerrit.

ctdinjeu3 6mo ago

[dead]