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:))
You must log in or register to comment.
If you were reviewing a “non-trivial” PR from me, I’d recommend not squashing because I would’ve broken it up into readable atomic commits.
This should be much more wide-spread. The hardest part of programming is reading someone else’s code.
More people should learn to do
git rebase -i, it’s a simple way to re-organise your commits to make sure that they tell a story to someone going through the PR commit by commit. It only takes a minute and can save your colleagues so much time and increase the quality of the review process.Or use a tool like StackedGit which makes the atomic commit workflow incredibly simple. Build atomic commits as you go instead of after you’ve written all of the code.
