Re: [PATCH v2 12/13] name-rev: eliminate recursion in name_rev()

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 27, 2019, 17:57 UTC
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.