> Edit: GitHub has similar functionality, but it doesn't know how to diff force pushes, so you would end up having to review the full diff again to ensure integrity.
I realize this is a religious war I'm stepping into, but I fall on the side of "if you are doing a force push of a branch already in code review you are doing something wrong". Once commits are out in the open and in review between multiple developers, that is shared code history that encodes important information about the code review process itself. (I make obvious exceptions for things like accidental PII disclosure or similar bone-headed mistakes, but the longer a branch has been in code review, the less willing I am to allow it to be force pushed.)
Of course, to each their own and your mileage may vary, but that comment about diffing force pushes in a merge/pull request sent a shiver down my spine, regardless of whether or not the tool supports it.
We often have to rebase our feature branches before they can be merged, so changing the history is common and force pushes are inevitable. As a rule I don't base any work on top of unmerged feature branches, because there is no expectation of stability.
I like the commit history to form a narrative of meaningful changes. I don't think commit messages about fixing errors that were introduced in the feature branch's previous commits belong in the history.
I like the ability to drill down into the details of why a change was made, the narrative of a PR and the back and forth flow can be hugely useful. The git DAG can be a very powerful way to encode both the high level narrative and the low level details.
The `--first-parent` option to git log and git annotate lets you stick to that high level narrative of just PRs/features that changed. (More tools should offer that as an option, it's probably my biggest request for GitHub's commit view, personally.)
It's a DAG for a reason, and it's powerful to take advantage of that, spending a lot of work (and potentially building a lot of footguns) forcing it to be a straight line seems counter-productive to me.
Phabricator is really nice here. The history of the code review process is maintained by Phab itself, you can amend all you want against an inflight code review, and upon landing it all becomes a single commit in master with a link to the code review in the commit message.
You’d hate our team—we force push to master (not just feature branches) all the time. It seems like anarchy at first but it’s not really that hard to deal with. Our situation is a bit unique though, and I doubt it would scale well beyond a small team.
Yes, that sounds like you are trying to maximize footguns to me. I do hope for your sake that no one manages to trigger one. (…and I'm personally very glad at this point with GitHub supporting branch restrictions and the ability to disable force pushes for master.)
I realize this is a religious war I'm stepping into, but I fall on the side of "if you are doing a force push of a branch already in code review you are doing something wrong". Once commits are out in the open and in review between multiple developers, that is shared code history that encodes important information about the code review process itself. (I make obvious exceptions for things like accidental PII disclosure or similar bone-headed mistakes, but the longer a branch has been in code review, the less willing I am to allow it to be force pushed.)
Of course, to each their own and your mileage may vary, but that comment about diffing force pushes in a merge/pull request sent a shiver down my spine, regardless of whether or not the tool supports it.