Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I've heard people argue for this strategy a few times but I am not convinced. Most projects I work on have feature branches that get squashed into a single commit (erasing the history of the branch). If we even have the case described by the author we would just do the three steps (refactor, new api, update) as 3 commits.

One thing that has been a solid practice for me is to avoid long-lived branches. That seems to be what leads to this kind of multi-stage commit scenario. Someone wants to work for several days (or worse, weeks) on a feature and then drop the whole thing all at once. I strongly prefer to move things into main every day (or multiple times a day). One way to do this is to use feature flags to allow work-in-progress commits to land without causing problems. This also has the advantage (if systems are set up correctly) to enable on dev and staging environments for testing. It is also a hop-and-skip jump away from blue/green rollouts.

I don't want ways to make big commits easier to review. I want to force the team to commit small changes early and often. I understand not everyone agrees.



The people I know who prefer stacked diffs argue that it makes it easier to integrate changes faster, no matter the size. Part of this is because unlike a PR on GitHub, you can land parts of a stack: to use the example from the article, if the "small refactor" diff is good to go, it can be landed without landing the "new api" and "migrate API users" diffs. Centering commits rather than branches has the effect of making for smaller overall commits rather than long-lived branches.


I'm not sure I understand your exact logic, so let me talk through a simplified scenario for arguments sake. Imagine each change takes 1 day. Imagine the team releases code to production every day. On Monday a developer picks up the task, does the refactor, puts up a PR and the refactor is shipped Tuesday morning. Tuesday he works on the api and it ships Wednesday. Wednesday he finishes the migration which ships on Thursday.

In the alternate scenario the entire stacked commits go out together on Thursday after the entire 3 days of work is completed. So I do not see how it would make it easier to integrate changes faster. This problem becomes worse, as you can imagine, if the second task in the stack takes a week vs. a day. I believe this kind of logic can be extended to any timescale.


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.


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.


> have feature branches that get squashed into a single commit (erasing the history of the branch)

Terrible.

It makes git-blame and git-bisect essentially unusable.

If you have a regression, git-bisect can help you narrow it down to a single patch. Because of that, you want to have fifty 160-line patches in the git history, for a particular feature, rather than one 8000 line patch.

If a given line of code looks fishy, you want git-blame (or a series of git-blame commands) to lead you to a 160-line commit, with its own detailed commit message, rather than to a 8000-line commit.

You also want to preserve the order of the original commits. Just reading through the individual commit messages, in order, a few years later, can be super helpful for understanding the original design. (Of course the original patch set has to be constructed in dependency order; it needs to compile at every stage, and so on. That's a separate development step that comes on top of just implementing the functionality. The code must be presented in logical stages as well.)




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: