From: Jonathan Tan <jonathantanmy@google.com>
Date: Wed, 27 Nov 2019 17:57:51 GMT
Subject: Re: [PATCH v2 12/13] name-rev: eliminate recursion in name_rev()
Message-ID: <20191127175751.165445-1-jonathantanmy@google.com>
In-Reply-To: <20191112103821.30265-13-szeder.dev@gmail.com>

> Note that this slightly changes the order of lines in the output of
> 'git name-rev --all', usually swapping two lines every 35 lines in
> git.git or every 150 lines in linux.git.  This shouldn't matter in
> practice, because the output has always been unordered anyway.

I didn't verify that the changing of order is fine, but other than that,
this patch looks great.

> This patch is best viewed with '--ignore-all-space'.

Thanks for the tip! I ended up unindenting the loop to see the changes
better, but I should have done this instead.

> -static void name_rev(struct commit *commit,
> +static void name_rev(struct commit *start_commit,
>  		const char *tip_name, timestamp_t taggerdate,
>  		int from_tag)

There are many changes from tip_name to name->tip_name in this function
that mean that tip_name is no longer used within this function. Should
this cleanup have been done in one of the earlier patches?

Apart from that, overall, this patch looks like a straightforward good
change. When we have a parent, instead of immediately calling name_rev()
recursively, we first add it to an array, and then (in reverse order)
add it to a priority queue which is actually used as a LIFO stack.