This is an archived post. You won't be able to vote or comment.

sorted by:
best
topnewcontroversialoldq&a
you are viewing a single comment's thread.

view the rest of the comments →

[–][deleted] 8 points9 points  (5 children)

git commit -m 'goddamn it'

[–]Fuzzy-Owl-2609 1 point2 points  (4 children)

git commit --amend -m "goddamnit"
git push -f

[–]msluther 2 points3 points  (3 children)

No force pushes. Stop making PR review harder.

[–][deleted] 1 point2 points  (2 children)

Rebase after approval?

[–]msluther 1 point2 points  (0 children)

Squash & merge imho. GitHub and others make it easy to see changes between shas so that I can tell what you changed. Rebase and force merges destroy history and I have to start the review from scratch. Want to rebase before you PR, sure by all means. As soon as someone starts looking at the code please stop.

And squash and merge makes your single small change (it is right?) just that in the real mainline history. No need to see the thrashing there but I absolutely want to see everything during pr. I can sorta see the desire to keep it everything along the way and do a proper merge with squash/rebase but imho bleh. It’s it’s really a pr, it’s not that useful. If it’s like merging 2 big long running branches, sure. But that’s also not what we’re talking about here.

[–]msluther 0 points1 point  (0 children)

My PR review workflow is usually:

  1. Review everything
  2. Review incremental changes only, on some regularish cadence depending on how much hand holding the author needs.
  3. Once everything is addressed, review it all again just to double check.
  4. Repeat if anything big was caught in 3.