4 ms·
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
by 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 agoSo it's a workaround for "even small PRs take too long to review"?
- Denvercoder9 3y agoNo, 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 agoIsn'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 agoI 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?
- from-nibly 3y ago> is just to get a second set of eyes on the details, run the CI checks and to fulfill the review requirements AKA feedback
- mpweiher 3y agoHow 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?
- liamfd 3y agoGitHub 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.
- Denvercoder9 3y agoThe 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.