All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: [PATCH] index-pack: fix allocation of sorted_by_pos array
Date: Tue, 07 Jul 2015 08:49:19 -0700	[thread overview]
Message-ID: <xmqqvbdw3r40.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <CACsJy8CsUu1zEnah9Ah3tQxk8N-xPpOBuV5TpQ4EB6+nyiDW3g@mail.gmail.com> (Duy Nguyen's message of "Tue, 7 Jul 2015 07:36:23 +0700")

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".

To me personally real- and final- mean about the same thing
(i.e. what is the real type of the object that is stored?) in the
context of this codepath.

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).

Thanks.

  reply	other threads:[~2015-07-07 15:49 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 [this message]
2015-07-07 16:06               ` Jeff King
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=xmqqvbdw3r40.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --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.