Hacker News new | ask | show | jobs
by saagarjha 2596 days ago
> I committed and immediately realized I need to make one small change!

I think it might be nice to add a disclaimer saying that this is not advisable if you've already pushed the code. Suck it up and make a new commit–don't rewrite public Git history.

6 comments

I think changing history is fine on feature branches that only you are working on, though. IMO the benefits of keeping commit history clean outweigh the cost of having to push with "--force".
This then discourages ad hoc teamwork because you can't touch feature branches other than the ones you own. And because the results of getting it wrong are hairy, people will tend to stay away just in case. It's a chilling effect.

Or, someone might have created another branch off your feature branch, because they depend on your work. Now you've creaed a time bomb for them when they try to merge their work after you've merged your alternative-history version of it. (and the failure mode is just weird, it takes experience to identify that all those seemingly nonsensical merge conflicts are result of this situation)

Etc. It just breaks a lot of things. The Git model and bad UI are already taxing enough to work with in your head, concurrently with your actual programming and domain cognitive load, that adding the uncertainty and multiplied complexity from having history rewritten around you is just a bad tradeoff.

(This may be different if the scenario is not a team, of course...)

> This then discourages ad hoc teamwork because you can't touch feature branches other than the ones you own. And because the results of getting it wrong are hairy, people will tend to stay away just in case. It's a chilling effect.

Do you frequently go messing with a branch assigned to single developers on your team without any sort of prior heads-up? And then surprise them with new commits next time they push/pull? I guess if you do that then none of you can ever force-push, and it's great if that works for your team, but I feel like generally people try not to interject and instead give a heads-up before messing with others' branches, after which the original dev knows others are involved and can then avoid force-pushing (or sync up when needed).

Not as often as we'd like to[1], but it happens regularly in reviews, or when casually working together on the same thing in, or when basing branches on other people's unfinalized work, etc. If a team member is used to a force push / rebase type of workflow, it invariably means that stuff will blow up becuause they forgot they had rebased, or they do a force push out of habit, etc. If you include rebase / force push in your daily workflow, you will invariably foul up with it and forget to make an exception for the teamwork.

Your proposed "heads up" way can work in theory, but it introduces too much friction and risk of error, and still leaves the situation in shambles when they have a rebased branch on their laptop that they haven't pushed yet. So you need to have a multi phase protocol that requires many interactions and even then the other guy may well forget about it and do a rebase & force push out of habit in the end.

[1] Well, "branch assigned to a single developer" isn't a thing, but most feature branches are finished by 1-2 people.

> but it happens regularly in reviews

But then you already know someone is reviewing that branch! So both of you avoid force-pushing.

> or when casually working together on the same thing in

Again, you both know multiple people are involved here... "casually working together" is the heads-up! You don't need another one.

> or when basing branches on other people's unfinalized work

Without any sort of hint to the guy working on that branch? How do you know it's even in a stable state to build on if you have no communication?

> or they do a force push out of habit

Yes it breaks if you screw up, but I mean then you just deal with it right? It's a dumb mistake just like any other silly mistake that can happen during committing, it's infrequent, it's on an ephemeral branch, and it's completely reversible. I don't see why someone occasionally mistakenly pushing the wrong thing (forced or otherwise) on a branch that isn't even going to exist for much longer is such a catastrophic event that you have to formulate your whole team's entire development process around avoiding that event 100% at all costs?

> Your proposed "heads up" way can work in theory, but it introduces too much friction and risk of error, and still leaves the situation in shambles when they have a rebased branch on their laptop that they haven't pushed yet. So you need to have a multi phase protocol that requires many interactions and even then the other guy may well forget about it and do a rebase & force push out of habit in the end.

Again, you don't need an explicit heads-up when you already know multiple people involved, and you can just deal with the occasional errors, as I mentioned. See above.

> Again, you both know multiple people are involved here... "casually working together" is the heads-up! You don't need another one.

Well, one of force-pushing or casually working together had better be a rare exception, otherwise you'll do both at the same time. I'd rather not make force-pushing part of my normal workflow, because casually working together has more value to me.

> Without any sort of hint to the guy working on that branch? How do you know it's even in a stable state to build on if you have no communication?

Anything committed / pushed is assumed working (we tend to compile before commit). If they need to rework their changes, they'll rework their changes, but the current state of their branch is almost certainly closer to the final state of their branch than the current state of master is.

> Yes it breaks if you screw up, but I mean then you just deal with it right? It's a dumb mistake just like any other silly mistake that can happen during committing, it's infrequent, it's on an ephemeral branch, and it's completely reversible. I don't see why someone occasionally mistakenly pushing the wrong thing (forced or otherwise) on a branch that isn't even going to exist for much longer is such a catastrophic event that you have to formulate your whole team's entire development process around avoiding that event 100% at all costs?

The trouble is it's viral. By the time you've figured out it's happened, other people have probably pulled from that branch; at best they've spent time merging in the force-pushed version. More likely they've done that and then built more work on top of it, so either both versions make it into mainline and everyone is resolving conflicts, or you have to ask that person to rebase their work and create another chance for the same problem to occur.

you just rebase on top of their changes. No biggie here. It does not matter one bit if their branch rewrites itself underneath.
Rebasing on rebased branches means everyone has to do the same conflict resolution again and again, and you're in trouble if one person decides to do it differently. Whereas when you merge, the first person does the conflict resolution and everyone else just picks it up.
But you can't safely rebase that branch! Having rebased it, you would have now broken it for others working on it. So this is a great example how the damage spreads and taints other branches around the history rewrite.

(And even arriving at the "ok i could fix this with rebase" diagnosis will have been painful and frustrating and eaten time & energy, and you can't be sure you got away with it before actually doing it and waiting if your teammates will come kick you in the nuts. or worse, silently spend a day untangling their work.).

It's just fundamentally unsound.

> But you can't safely rebase that branch!

Huh? You don't rebase their branch. You rebase your own changes on top of their branch, which they happened to recently rewrite. Just like you might rebase your changes on top of master after master has undergone changes. I think the parent's point was that it doesn't matter if the branch was rewritten or just extended; either way you rebase the same commits on it the same way. (If you're one of those people who's against the notion of rebasing entirely then that's a separate debate we can have another time, but you need to separate that from the force-push issue.)

The "own" branch is also public here and maybe collaboratively worked on
Like a few other replies, I force push on my branches all the time.

I push my code up to my origin as soon as I can and as I go I’ll fix up my commits and force push.

There are some advantages for me anyway. Pushing to origin kicks off some smoke tests and end to end tests that are fairy slow and cumbersome to run on my dev machine. That helps me catch bugs earlier, especially since I’m working on a Microservices architecture. Also it acts as a backup for if my dev machine dies on me. I prefer to fix up and force push to create a clean logical story from my code rather than leave in spurious commits which exist only to fix linting for example.

I find it okay to rewrite public history in short lived feature branches. It’s typically your branch after all.

So it would change your advice to: Don’t rewrite public Git history unless you can assume it’s read only.

Patches for Linux get rewritten all the time till they are finally merged.

That's not public git history though, those are just patches on mailing lists. Even before it goes into maintainer's lists, in my experience it's extremely rare that a maintainer will force push to their own public -next branch.
EDIT: parent is right.

I see about 600 pull requests on github[1]. My understanding was that Linus moved away from mailing lists some time ago. But then I'm not involved in kernel hacking.

[1] https://github.com/torvalds/linux/pulls

The Linux kernel has a bot that reminds people that kernel development happens on the mailing list: https://github.com/KernelPRBot. Since GitHub does not allow for disabling the pull requests tab, the pull requests invariably get closed.
Did you ever look at any one of these PRs?
I force-push all the time in my feature branches. It's expected. If I'm using `git commit --fixup` and `git rebase -i origin/master --autosquash` that feature branch is going to come out clean and nice in the merge commit if I force push to my feature branch.
> Suck it up and make a new commit–don't rewrite public Git history

You're most likely making things worse. Not only will you have the mess-up commit(s), but also the un-mess-up revert-commit. It gets super hairy when reverting merges (I assume you're not rebasing + fast-forwarding if you lobby against rewriting).

There is no big deal in rebasing to a rewritten master branch. It's just like any other rebase.

IMHO, rebasing signals that someone wants to apply a patch, and knows exactly where. Merging/reverting/etc. signals someone wants to "upload his latest stuff", like to a dropbox.

This is indeed a job for git revert, push/pr, check out a new branch then revert the revert with better messaging :)