Comment by palunon
7 years ago
>If you're having to fix stuff with -f, you're going to run into trouble; try to avoid -f ever.
I disagree. I don't have much experience using git with big teams, but at least for small team where a developer usually owns a feature branch, force pushing to the branch to take into account criticism on commits can be helpful. Of course, in that case, feature branches are considered non shared.
Git own "next" branch is actually force pushed to all the time, to remove patches which didn't make the cut for example.
Personal opinion - re-reviewing a pull request when someone has erased all context from your last review of that PR via a rebase and force push is a royal PITA. Especially for changes that span more than a few files. Did they address your concerns? The only way to know is to go back through all of the changes on that feature branch, and hope you don't miss anything.
This, in contrast with just reviewing the latest commit, potentially going back to the rest of the changes with an eye towards the latest commit.
It frankly bugs the piss out of me when people rebase and ask me to review their latest changes - it feels disrespectful of my time.
I've taken to making the change as a fixup commit and leaving it in the PR for code review. Then when the changes are approved a quick `git rebase -i --auto-squash` cleans everything up before merging the branch back into `develop`.
> force pushing to the branch to take into account criticism on commits can be helpful
Hm? Criticism on commits should be additive. The commit was made, response was made, and a new commit addresses the response. Rewriting history breaks a lot of things and removes context. Another approach:
1) Add commit addressing criticism.
2) Squash when merging feature in to master, which is one of the only cases where I think rewriting history is OK.
Much cleaner master history, no rewriting history (which you should never do once another checkout sees your branch, including GitHub), and a number of other benefits. If you're routinely force pushing feature branches, something is broken in your workflow, IMO.