Re: [PATCH 1/2] Change "refs/" references to symbolic constants

[Jeff King <peff@peff.net>]

On Tue, Oct 02, 2007 at 09:41:03AM +0100, Andy Parkins wrote:

> Excellent!  Well done.  I spent a couple of hours last night going through 
> every changed line and have spotted the TAGS mistake but didn't spot the 
> STRLEN being wrong.  Amazing how easy it is to become blind to these things.  
> There were a couple of errors in "/" placement too, but I don't think they 
> were causing any trouble, just doubled up "/" characters.

I wonder if you could check the patch mechanically (i.e., write a script
to confirm that '5' got replaced by tokens equal to '5'), though that
might require some tricky parsing of C.

If you a post an updated version, I will try to read through it
carefully, as two eyes are better than one (er, four eyes, I guess).

> I noticed a couple of places where memcmp() has been used where prefixcmp() 
> would work fine.  I'm tempted to change them too - what do you think?  
> Perhaps a separate patch?

When in doubt, I would suggest a separate patch, as it makes the review
easier.

> I'm happy to do prepare a patch against any revision, I was really
> waiting for feedback from Junio as to how he'd like to manage it.
> Last time I submitted this patch he (quite correctly) asked that I
> delay until after the next point release; of course I promptly found
> other things to do and never resubmitted :-)

Yes, you should definitely listen to Junio on such issues, and not me.
:)

-Peff