From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Duy Nguyen <pclouds@gmail.com>, Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] index-pack: fix allocation of sorted_by_pos array
Date: Tue, 7 Jul 2015 12:06:31 -0400 [thread overview]
Message-ID: <20150707160630.GA4456@peff.net> (raw)
In-Reply-To: <xmqqvbdw3r40.fsf@gitster.dls.corp.google.com>
On Tue, Jul 07, 2015 at 08:49:19AM -0700, Junio C Hamano wrote:
> 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.
> 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
next prev parent reply other threads:[~2015-07-07 16:06 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-04-18 10:47 [PATCH 0/2] nd/slim-index-pack-memory-usage update Nguyễn Thái Ngọc Duy
2015-04-18 10:47 ` [PATCH 1/2] index-pack: reduce object_entry size to save memory Nguyễn Thái Ngọc Duy
2015-04-18 10:47 ` [PATCH 2/2] index-pack: kill union delta_base " Nguyễn Thái Ngọc Duy
2015-07-03 16:51 ` Junio C Hamano
2015-07-03 18:29 ` Duy Nguyen
2015-07-04 1:21 ` [PATCH] index-pack: fix overallocation of sorted_by_pos array Junio C Hamano
2015-07-04 22:30 ` [PATCH] index-pack: fix allocation " Junio C Hamano
2015-07-06 23:23 ` Junio C Hamano
2015-07-07 0:36 ` Duy Nguyen
2015-07-07 15:49 ` Junio C Hamano
2015-07-07 16:06 ` Jeff King [this message]
2015-07-08 11:56 ` [PATCH 1/2] index-pack: rename the field "type" to "in_pack_type" Nguyễn Thái Ngọc Duy
2015-07-08 11:56 ` [PATCH 2/2] pack-objects: rename the field "type" to "real_type" Nguyễn Thái Ngọc Duy
2015-07-08 13:47 ` Jeff King
2015-07-08 13:57 ` Duy Nguyen
2015-07-08 14:11 ` Jeff King
2015-07-04 22:24 ` [PATCH 2/2] index-pack: kill union delta_base to save memory Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150707160630.GA4456@peff.net \
--to=peff@peff.net \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=pclouds@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.