Hacker News Reader

Comment by steveklabnik | Hacker News Reader

Do you only send PRs with one commit in them every time?

I find in my work that if I were doing a three-commit series like this, I would end up sending in a PR on Thursday, containing all three commits. This is because I can’t be sure the refactor works well until I use the results of it with the new code, for example.

In the scenario I’m talking about, to adapt it to yours, both developers send out their PR or stack on Thursday. Later that day, the refactor commit is approved, but the new API is still under discussion. That discussion takes a full day to get signed off. On Friday, the full PR/stack has been reviewed and all changes are ready to land.

In this scenario, the refactor lands from the stack on Thursday, and the rest lands Friday. In the PR world, both end up landing Friday, because the refactor, as part of the PR, doesn’t land until the whole PR does.

View on HN

Every PR is a single commit. The only extra commits on a PR would be "Review feedback", etc. But it would be squash merge and the final commit message updated to include any important details related to the changes from the feedback.

Consider the case from the article. refactor: 25, api: 500, migration: 50. Lets say 25 = 1 day of work so we have refactor: 1 day, api: 20 days, migration: 2 days. I'll decrease api to 5 days (1 week) for simplification. You pick up the task on Monday and do the refactor. Now, you have 5 days of work ahead of you on the api. Don't you feel you are looking over your back hoping no one on your team touches those lines? And then on day 5 someone does the refactor but slightly different. You have a commit so you have to do some rebase magic when you catch up to master. No thank you. And you doubled the work, your coworker wouldn't have had to do it if you had checked it in 5 days ago.

Consider a deadline. Consider a major production bug in the api that you discover when you release it. In my plan, you end up releasing the api early so you catch the bug early. You can petition for more resources to fix the problem since you have extra days which were reserved for the migrarion. If you try a big-bang release you find the api bug later, decreasing the likelihood that you can fix it within the deadline. I think of amortizing the risk of my releases across time rather than allowing the risk to accrue.

I could continue to defend this with more examples but the real fact is there is a tradeoff here like tabs vs. spaces or emacs vs. vim. But that tradeoff is ship fast vs. reduce interruptions for developers. Because creating a PR, reviewing a PR and releasing a change are interruptions for developers. And some developers will find ways to reduce interruptions. I have my preference for shipping fast for the reasons above and my experiences between cultures that result from the choice of prioritization in the tradeoff.

You’re actually making the same argument that the article is—it’s just hard to tell because terms are being used differently.

Where you talk about merging PRs early, the article talks about merging commits early: “You don't have to wait on all 5 commits to be ready to go; maybe the first 3 are OK, and the last 2 need more work. You merge 3 out of 5.”

But what you call a disruption-reducing tradeoff, the article argues is just shitty UX on GitHub’s part. The stacking-native tools it mentions are what you’d get if you took the workflow you propose and let devs start task B (and then maybe even C) immediately after task A, instead of making them wait for reviewer sign-off on A.

Yeah, as my sibling says, you’re making the same argument the article is, it’s just that apparently for you, reviews and merging happen so quickly you never have outstanding PRs depend on each other, so you never need to stack. Treating the commit as the unit of review than a branch as the unit of review is the more fundamental difference, and you’ve got that.

Reply on news.ycombinator.com