Re: [PATCH] index-pack: fix allocation of sorted_by_pos array
- From
Jeff King <peff@peff.net>
- Date
- Jul 7, 2015, 16:06 UTC
- Message-ID
- <20150707160630.GA4456@peff.net>
- In-Reply-To
- <xmqqvbdw3r40.fsf@gitster.dls.corp.google.com>
On Tue, Jul 07, 2015 at 08:49:19AM -0700, Junio C Hamano wrote:
Show 15 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes: > > > 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.
Show 9 quoted lines
> 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