Re: [PATCH v4 4/4] Accept tags in HEAD or MERGE_HEAD

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

2011/8/20 Junio C Hamano <gitster@pobox.com>:
> That's de-reference, not deference ;-). You may want to be more explicit
> about what kind of de-reference you are talking about.
>
> /*
>  * Get a commit object for the given sha1, unwrapping a tag object that
>  * point at a commit while at it. ref_name is only used when the result
>  * is not a commit in the error message to report where we got the sha1
>  * from.
>  */
>
> I actually was hoping that you would have this comment in commit.h to help
> people who want to add callers of this function, not next to the
> implementation.

OK. It's because I tend to go straight to implementation instead of
the declaration when I want to know how to use it.

> As I said earlier, I do not think updating sha1[] here is necessary. The
> caller should be updated to use c->object.sha1 instead.

The usual pattern is

get_sha1(ref, sha1);
commit = lookup_commit_or_die(sha1, ref);

From a quick look, it's very easy to assume "sha1" is safe to use
afterwards while it may be different from commit->object.sha1. I'm
tempted to make lookup_commit_or_die() resolve ref to sha1 internally,
no temporary sha1 variable will be hanging around, the pattern becomes

commit = lookup_commit_or_die(ref);

The only problem is MERGE_HEAD is not usual ref and cannot be treated this way.
-- 
Duy