Re: [PATCH] index-pack: fix allocation of sorted_by_pos array

On Tue, Jul 07, 2015 at 08:49:19AM -0700, Junio C Hamano wrote:

Duy Nguyen [off-list ref] writes:

quoted

I keep tripping over this "real_type vs type" in this code. What do
you think about renaming "type" field to "in_pack_type" and
"real_type" to "canon_type" (or "final_type")? "Real" does not really
say anything in this context..

An unqualified name "type" does bother me for the word to express
what representation the piece of data uses (i.e. is it a delta, or
is it a base object of "tree" type, or what).  I think I tried to
unconfuse myself by saying "representation type" in in-code
comments, reviews and log messages when it is not clear which kind
between "in-pack representation" or "Git object type of that stored
data" a sentence is talking about, and I agree "in_pack_type" would
be a vast improvement over just "type".

I think this is doubly confusing because pack-objects _does_ use
in_pack_type. And its "type" is therefore the "real" object type. Which
is the opposite of index-pack, which uses "type" for the in-pack type.
So at the very least, we should harmonize these two uses.

Especially, if the other one is renamed with "in_pack_" prefix,
"real_type" is not just clear enough but is probably better because
it explains what it is from its "meaning" (i.e. it is the type of
the Git object, not how it is represented in the pack-stream) than
"final_type" that is named after "how" it is computed (i.e. it makes
sense to you only if you know that an in-pack type "this is delta"
does not have the full information and you have to traverse the
delta chain and you will finally find out what it is when you hit
the base representation).

Yeah, I agree real_type is fine when paired with in_pack_type. We might
consider modifying pack-objects.h to match (on top of just moving
index-pack to in_pack_type).

-Peff