For non-trivial reviews, when there are files with several changes, I tend to do the following in Git:
- Create local branch for the pr to review
- Squash if necessary to get everything in the one commit
- Soft reset, so all the changes are modifications in the working tree
- Go thru the modificiations “in situ” so to speak, so I get the entire context, with changes marked in the IDE, instead of just a few lines on either side.
Just curious if this is “a bit weird”, or something others do as well?
(ed: as others mentioned, a squash-merge and reset, or reset back without squashing, is the same, so step 2 isn’t necessary:))
Squashing seems like you’d potentially lose out on info and have a harder time isolating the changes you’re looking through. I guess it depends on how much has been changed and whether some of the commits along the branch were more important than others.
I also don’t think the reset is necessary, you should be able to diff the branch head against whatever you want.
Yeah I had the same thoughts, had no idea people even bothered to do that
Sometimes the info lost is just a typo or a revert. I’d say heavily depends on the workflow of the people involved. Some like long history, some like rebasing, others, something in between. How you review those approaches changes a lot
Sure, that’s fine. I use interactive rebase for “cleaning” a lot. I’m just saying it doesn’t make a difference for diffing (as you can diff any commit against any other) and doing it as a matter of routine sounds like it could skip potentially useful history.
I mostly rebase but if a branch has things happen in a sequence that matters, I would merge it instead, for example.