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

[Nicolas Pitre]

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.

In the above example, it is still possible that lp666xxx was forked from 
an earlier point from where your HEAD currently is.

 a ----- b ----- c ----- d ----- e  <-- HEAD
                  \
                   f ----- g <-- FETCH_HEAD

If you do 'git log -p HEAD..FETCH_HEAD' all you'll see is f and g which 
is fine.  But doing 'git reset --hard FETCH_HEAD' here means that your 
HEAD will be forcefully moved to the same position as FETCH_HEAD, 
effectively making commits d and e unreachable and therefore excluded 
from your tree.  That is most probably not what you want.

> > If you're hunting for a merge base, then something is likely wrong.

Why is that so?

> > We generally are not dealing with wildly divergent trees.

You don't need wildly divergent trees.  Git is distributed, so people 
might be basing their work on a different base than what you have 
locally.

> I wrote this up almost exactly as I got the instructions from Stephan.
> Note, I'm not saying you are wrong here.

I do.  :-)


Nicolas