DEV Community

Discussion on: The Case Against Pull Requests (And How to Fix Them)

Collapse
 
simonhaisz profile image
simonhaisz

Alternatively, just create small PRs that always target master?

Creating and switching branches takes seconds so I'm struggling to understand how that can be considered a burden or a big overhead unless you were doing it thousands of times a day. I think the most PRs I've submitted in a single day was 10 and I can imagine a workflow where someone might need to do 20+. That's maybe 5 minutes of their day switching between branches? Seems like there are likely better opportunities to improve productivity to me.

Collapse
 
shubhamjain profile image
Shubham Jain • Edited

Consider this scenario: you've filed a PR and moved on to the next one. Then, you realize there could be some improvement in the last PR. You would have to checkout the branch again and create a new commit and checkout the last branch again. If you're working on the same branch you would just have to amend the last commit. Another case could be where your PR is not merged and then, you're working on another which might have conflicting changes.

I agree that the cost of creating branches is not that high, but it's the context switching that can be a time sink. I think Google's workflow is about worrying mostly about your change and little else.

Collapse
 
simonhaisz profile image
simonhaisz

Your first example has you making no changes locally after creating the PR. You would only be switching to a new branch if you had other work you needed to do. If we assume a model of "I'm going to stay on this work item until its in master", which it looks like to me you are, you would just stay on this branch until your PR is approved. If there is feedback that requires changes you make them and push to the remote PR branch. If your teams doesn't like the fact that there are multiple commits, can amend/squash locally and force push to your PR branch. I have some co-workers who feel strongly about re-writing history so that their merges are clean (single commit), which fits this case.

And if you are working under this model on work items in serial (one after the other, never moving on to the next item before the previous is finished) then you can accomplish that without local branches, only remote PR branches. You can do all of your work on your local master branch but when you push, instead of pushing to remote/origin/master you can go to remote/origin/shubham/JIRA-1337.

As to your second example, you can avoid conflicting with yourself by using PR chaining. I do this sometimes when I have a series of changes that depend upon each other. After the first change I create PR-A that points to master. Then the second is PR-B pointing to PR-A, and so one. This allows my reviewers to review my changes in bite-size chunks and unblocks me from having to wait for them to review each change before starting on the next. If I need to respond feedback on a particular PR I can easily switch to that branch and make changes. The downstream PRs can then get updated appropriately.

Now, obviously the above can easily get complicated but I consider that complexity 'worth it' because that's how I stay productive. My reviewers are not going to drop everything they are doing and review my work immediately - that wouldn't be good for their productivity with context switches outside of their control. Sometimes PRs will get reviewed within a few minutes of creation and that's great! But other times is could take a few hours before they have a chance.

So if you live in a world where you are making dozens of commits a day and getting immediate, useful feedback on each change I would get your position. I've just never lived in a world anything like that before :)

Collapse
 
yawaramin profile image
Yawar Amin

it's the context switching that can be a time sink

But in the commit review world you'll be constantly context-switching as your teammates push new commits and you have to drop everything and review them to unblock them from pushing their next commits. Especially for a smaller team and for teams with more juniors or newer teammates (who tend not to review other people's PRs), this will bog down everyone's commits in a big queue waiting to be reviewed.