Re: [PATCH 1/2] describe: Use for_each_rawref

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 7, 2010, 21:59 UTC
Message-ID: <7vr5dtnt0h.fsf@alter.siamese.dyndns.org>
In-Reply-To: <7vsjy9pdmg.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 14 quoted lines

> Anders Kaseorg <andersk@ksplice.com> writes:
>
>> On Tue, 7 Dec 2010, Anders Kaseorg wrote:
>>> Signed-off-by: Anders Kaseorg <andersk@ksplice.com>
>>> 
>>> diff --git a/builtin/describe.c b/builtin/describe.c
>>
>> (Gaaah, sorry, I accidentally used format-patch -p here.  Won’t happen 
>> again.  :-) )
>
> The default will work just fine, no?
>
> Also labelling them like "[PATCH v2 1/2]" would have been nicer to spot
> the progress of the series.

Two more requests.

 * Please describe what was changed since earlier interations after "---"
   lines;

 * Please run tests before submitting patches.  It appears to break t7407.

What breaks in t7407 is "git submodule status --recursive" output. In my environment, it used to describe the tip as heads/master but after the patch it says remotes/origin/master. The output from "describe --all" does not tiebreak between one branch from another, even though it does favor tags over branches (and tries to use annotated ones), so it is not a real regression in that sense, but then the test needs to be fixed not to care about which equally good branches are shown.

Some might even argue that it is more relevant to show how the commit relates to remote-tracking branch heads. That is a separate issue (if this position is widely supported, we would introduce another "prio" between "all others (0)" and "lightweight tags (1)" for remote-tracking branches), though.