On Tue, Jul 07, 2015 at 08:49:19AM -0700, Junio C Hamano wrote: > Duy Nguyen <pclouds@xxxxxxxxx> 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. > 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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html