Re:Code Reviews and Bad Habits
by jrockway on Thursday February 20, 2014 @03:42PM
(#7273409)
I disagree with this; you make one patch for your reviewer to review. If you want to do two things, make two patches.
The history of how you made that patch should be irrelevant; if you care, your patch is too big. Split it up.
I will admit that this takes some getting used to. When I use git alone, I commit for every little thing I change. But that's not actually useful for other people to look at; what people really want are high-level patches that represent logical changes, not just physical edits of the source code.
I've also gotten myself into a lot of trouble by committing a lot of things and then splitting them up for review. This always takes a very long time.
So just focus on one thing at a time, send it for review, and make sure your coworkers believe that reviews are the most important thing they can be doing. Then you get the small commits you like, but also get a more readable history, and better feedback from reviewers.
The history of how you made that patch should be irrelevant; if you care, your patch is too big. Split it up.
I will admit that this takes some getting used to. When I use git alone, I commit for every little thing I change. But that's not actually useful for other people to look at; what people really want are high-level patches that represent logical changes, not just physical edits of the source code.
I've also gotten myself into a lot of trouble by committing a lot of things and then splitting them up for review. This always takes a very long time.
So just focus on one thing at a time, send it for review, and make sure your coworkers believe that reviews are the most important thing they can be doing. Then you get the small commits you like, but also get a more readable history, and better feedback from reviewers.