Re: [PATCH v2 15/20] for_each_object_in_pack(): convert to new revindex API

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 14, 2021, 20:51 UTC
Message-ID: <xmqqv9bzw8bb.fsf@gitster.c.googlers.com>
In-Reply-To: <YACcoNY/SiEbBSgh@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 14 quoted lines

>  	for (i = 0; i < p->num_objects; i++) {
> -		uint32_t pos;
> +		uint32_t index_pos;
> ...
>> *1* The nth_packed_object_id() call we make later using the value we
>> obtain here should be documented to take "index" as its last
>> parameter, now that is what we call the location in the index, which
>> is in object name order.
>
> I would love to see the function given a more descriptive name. Having
> worked on the bitmap code a lot, where the norm is pack-order, saying
> "nth" is confusing and error-prone.
>
> But I think that's out of scope for this series.

Yeah, an explicit index_pos (vs pack_order_pos) would be good names to use, and nth_packed_object_id() can also use somewhere in its name to hint that it is about the object name order, but I agree that both are outside the scope of this series.

Thanks.