[Ubuntu Wiki] Update of "Kernel/Dev/HowtoReviewPullRequests" by brad-figg

[Nicolas Pitre]

On Fri, 11 Jun 2010, Henrik Rydberg wrote:

> Nicolas Pitre wrote:
> > On Thu, 10 Jun 2010, Brad Figg wrote:
> > 
> >> On 06/10/2010 12:00 PM, Tim Gardner wrote:
> >>> When I review a pull request it generally goes something like this:
> >>>
> >>> git fetch git://kernel.ubuntu.com/bradf/ubuntu-lucid-wiki lp666xxx
> >>> git log -p HEAD..FETCH_HEAD
> >>>
> >>> If I like what I see I either cherry-pick the chosen commits, or in
> >>> simple cases 'git reset --hard FETCH_HEAD', then push.
> > 
> > I'd really recommend _not_ doing this.  Using "git reset --hard" isn't 
> > something you should be using lightly.
> 
> Hello, I am really just a bystander regarding this, but as someone using git
> daily, perhaps a second voice might be of use, anyways.

Sure.

But just that people don't think I'm talking through my hat, I'm a 
daily Git user as well as a Git developer.

> There is nothing wrong with using git reset --hard. In my experience, git forces
> you to know what you are doing, which IMHO is a good thing. Compared to the
> alternative, which could for instance be fuzzy merges, which puts your local
> tree somewhere in in limbo between the origin and what you would like it to be,
> using reset --hard gives you control. You know _exactly_ what sha1 applies to
> your tree.

But that's the problem.  To use 'git reset --hard' you must know what 
you're doing, which is best left to expert Git users and not something 
you should expect the kind of people relying on a HowTo to do.  Because 
"git reset --hard" is bypassing a lot of safety measures which would 
otherwise prevent you from unknowingly throwing commits away.

> In my mind, the "git pull" command was a mistake, since most contributors would
> ever only want to do "git fetch" follows by "git rebase", to avoid those weird
> merges.

There is "git pull --rebase" for that if you wish, which is smart enough 
to find out the proper rebase boundaries in case you did a fetch more 
than once.

I don't necessarily agree that "rebase" is always the good thing to do.  
Sometimes it is but not always.


Nicolas