This blog post exactly describes the workflow of Gerrit, except that Gerrit handles most of this workflow for you, especially in the case where PART1 or something needs updates in response to code review.
GitHub adopting the Gerrit model would be amazing. In the meantime, boy oh boy this is such an advertisement for Gerrit.
I don’t really see the point of the initial pull request. A pull request is a request for changes to be pulled, but if you don’t want changes to be pulled, creating a pull request seems wrong. If it is something to do with being able to ‘review’ code through GitHub’s pull request screen, I recommend avoiding trying to do that: every experience I’ve had trying to review code through GitHub’s pull request/review feature was a total nightmare honestly.
This just seems like a really unwieldy way of emulating a patch series. This is why mailing list-based git collaboration makes so much more sense than the ‘pull request’ system: you end up emulating the features of mailing lists anyway. If your work flow involves reacting to email notifications about changes to ‘stacked pull requests’ then I’d suggest that you might want to investigate a work flow where people just use email to respond to a patch series.
Even if you insist on using GitHub’s pull requests, why not just review the changes commit-by-commit instead of breaking each commit out into its own separate pull request?
First of all, without seeing the code behind the PRs and commits, it’s difficult to know what conclusion to draw. Maybe the stacked PR would have had more discussion in either case?
Perhaps the underlying problem is the PR “… contains multiple logical changes,” implying it should have been multiple PRs in the first place.
One of my co-workers is really good about making small, self-contained commits, and when I review his PRs I go through commit-by-commit and follow along with the changes over time. It seems to give all of the benefits of these “stacked pull requests”, without the overhead, and it makes reviewing larger PRs much easier. I’ve been trying to get better at it and use the same technique when I make large changes.
I think git send-email would indeed work fine for most users. However git am would be where you would loose most devs because of the challenges that exposes with their existing email setup.
There’s no need for a complicated email setup, maintainers can just copy the link to the raw message in the mailing list archives and do curl <link> | git am.
I admit that I don’t have much experience in a ‘professional’ setting, but at first glance, this seems unwieldy. Submitting diffs in that many pieces, and requesting independent review seems like it would require MORE cognitive load to keep track of. Also, the ‘seems fine’ comment on the monolithic PR seems like a straw man.
I used a similar workflow in mercurial and quite liked it - but the big difference was if I added a commit to one of the base branches, I could merge/rebase the entire stack of branches at once.
Is there an easy way to do that in git instead of having to go through the git checkout/git merge dance for each stacked branch whenever a base is modified?
This blog post exactly describes the workflow of Gerrit, except that Gerrit handles most of this workflow for you, especially in the case where PART1 or something needs updates in response to code review.
GitHub adopting the Gerrit model would be amazing. In the meantime, boy oh boy this is such an advertisement for Gerrit.
I don’t really see the point of the initial pull request. A pull request is a request for changes to be pulled, but if you don’t want changes to be pulled, creating a pull request seems wrong. If it is something to do with being able to ‘review’ code through GitHub’s pull request screen, I recommend avoiding trying to do that: every experience I’ve had trying to review code through GitHub’s pull request/review feature was a total nightmare honestly.
This just seems like a really unwieldy way of emulating a patch series. This is why mailing list-based git collaboration makes so much more sense than the ‘pull request’ system: you end up emulating the features of mailing lists anyway. If your work flow involves reacting to email notifications about changes to ‘stacked pull requests’ then I’d suggest that you might want to investigate a work flow where people just use email to respond to a patch series.
Even if you insist on using GitHub’s pull requests, why not just review the changes commit-by-commit instead of breaking each commit out into its own separate pull request?
Interesting point of view, thank you. I’ll delve into this a bit more. But I’m affraid that patch by email isn’t used for many projects.
It should be!
https://drewdevault.com/2018/07/02/Email-driven-git.html
https://drewdevault.com/2018/07/23/Git-is-already-distributed.html
https://github.com/torvalds/linux/pull/17#issuecomment-5654674
https://git-send-email.io/
Thanks, I found the first link a minute ago ;-)
There is a way to create draft pull requests on github, which can’t be merged.
I’m not sure I buy it.
First of all, without seeing the code behind the PRs and commits, it’s difficult to know what conclusion to draw. Maybe the stacked PR would have had more discussion in either case?
Perhaps the underlying problem is the PR “… contains multiple logical changes,” implying it should have been multiple PRs in the first place.
One of my co-workers is really good about making small, self-contained commits, and when I review his PRs I go through commit-by-commit and follow along with the changes over time. It seems to give all of the benefits of these “stacked pull requests”, without the overhead, and it makes reviewing larger PRs much easier. I’ve been trying to get better at it and use the same technique when I make large changes.
Also wondering if this flow would be done differently in 2020.
[Comment from banned user removed]
I think
git send-emailwould indeed work fine for most users. Howevergit amwould be where you would loose most devs because of the challenges that exposes with their existing email setup.There’s no need for a complicated email setup, maintainers can just copy the link to the raw message in the mailing list archives and do
curl <link> | git am.I admit that I don’t have much experience in a ‘professional’ setting, but at first glance, this seems unwieldy. Submitting diffs in that many pieces, and requesting independent review seems like it would require MORE cognitive load to keep track of. Also, the ‘seems fine’ comment on the monolithic PR seems like a straw man.
Having a lot of experience with participating in and/or leading teams using PR workflows in professional shops: it’s absolutely not a straw man.
Could be, but that is how I experienced it in a professional setting.
I used a similar workflow in mercurial and quite liked it - but the big difference was if I added a commit to one of the base branches, I could merge/rebase the entire stack of branches at once. Is there an easy way to do that in git instead of having to go through the git checkout/git merge dance for each stacked branch whenever a base is modified?