Re: [PATCH v3] diff-no-index: exit(1) if 'diff --quiet <repo file> <external file>' finds changes

From: Jeff King <peff@peff.net>
Date: Jun 20, 2012, 16:06 UTC
Message-ID: <20120620160607.GA12856@sigill.intra.peff.net>
In-Reply-To: <CAFouetgXkqJPYwjr5ob5ed_ooL-D56zXyjnOAWrVPdt_eZqw7g@mail.gmail.com>

On Wed, Jun 20, 2012 at 09:38:15AM -0400, Tim Henigan wrote:

Show 12 quoted lines

> > With your patch applied, I was able to use relative paths in my tests.
> >  I also confirmed that all the t4*.sh tests pass.
> >
> > For what its worth, your patch looks correct to me.  Existing
> > consumers of 'prefix_path' should get the same results as before and
> > the one added xmalloc is paired with a free.
> 
> Jeff,
> 
> Are you planning to send this patch to the list?  If not, can I
> include it as 1 of 2 before my patch?  If we go that route, I'm not
> sure how to properly show you as the author...

I'd probably get to it eventually, but I haven't touched it since I sent it. If you want to include some tests and package it with a commit message, that would make me very happy.

You can override the author by including a "From: " header as the first line in the body of the email (which git-am will use rather than the identity in the email's From header). If you use git-send-email, it will do this automatically when the patch author does not match your identity.

I didn't sign-off the original, but please feel free to include my sign-off, as well as add your own. And note your own contributions in the commit message. So the resulting email would be something like:

   From: Tim Henigan <tim.henigan@gmail.com>
   Date: ...
   Subject: [PATCH 1/2] diff: handle relative paths in no-index

   From: Jeff King <peff@peff.net>

   ... some commit message body ...

   Tests and commit message by Tim Henigan.

   Signed-off-by: Jeff King <peff@peff.net>
   Signed-off-by: Tim Henigan <tim.henigan@gmail.com>
   ---
   ... the actual patch ...

> Also, in an earlier email [1] you mentioned that it may be a good idea
> to rename 'found_changes' to something like 'xdiff_found_changes'.  I
> like the idea...I could submit this change as another patch in the
> series, if you have no objections.

Fine by me. I think "xdiff_found_changes" is not quite accurate; it is really "did builtin_diff find any changes?" since we might never call into xdiff (e.g., for binary files). I'm not sure what the best name is.

-Peff