All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Victoria Dye <vdye@github.com>
Cc: Shuqi Liang <cheskaqiqi@gmail.com>,
	git@vger.kernel.org, derrickstolee@github.com
Subject: Re: [RFC][PATCH V1] diff-tree: integrate with sparse index
Date: Tue, 16 May 2023 16:14:05 -0700	[thread overview]
Message-ID: <xmqqsfbvswci.fsf@gitster.g> (raw)
In-Reply-To: <fa24482c-7c48-9b7f-5d97-3dbf9822728c@github.com> (Victoria Dye's message of "Tue, 16 May 2023 14:19:04 -0700")

Victoria Dye <vdye@github.com> writes:

> Longer version: 

Thanks, as usual, for a great review.  A lot of the stuff you wrote
should inspire and result in an improved log message that explains
why this change is sufficient to teach diff-tree to take advantage
of the sparse index.

> However, 'DIFF_SETUP_USE_SIZE_CACHE' seems to be unused as of 6e0b8ed6d35
> (diff.c: do not use a separate "size cache"., 2007-05-07) and nothing about
> 'detect_rename' otherwise indicates index usage, so AFAICT that whole
> condition can be dropped (along with DIFF_SETUP_USE_SIZE_CACHE,
> DIFF_SETUP_REVERSE, and diff_options.setup).

True.  The size cache does not exist anymore.  6b5ee137 (Diff
clean-up., 2005-09-21) restructured the command line option parsing
quite a bit, and we lost DIFF_SETUP_REVERSE, which is a bit that
gets OR'ed in to a file-scope diff_setup_opt static of each of the
command in the diff family.  The bit and the diff_setup_opt variable
got replaced with members of "struct diff_options", and I should
have removed the macro at the same time.

> Note that, if you want to make
> that change in this series, it should be done in a separate patch _before_
> this one (since dropping the deprecated setup infrastructure isn't really
> part of the sparse index integration).

Or after this one, perhaps?  I agree that the clean-up opportunity
you found is very much unrelated to the work to teach diff-tree to
take advantage of the sparse index.

THanks.

  reply	other threads:[~2023-05-16 23:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-15 19:18 [RFC][PATCH V1] diff-tree: integrate with sparse index Shuqi Liang
2023-05-16 21:19 ` Victoria Dye
2023-05-16 23:14   ` Junio C Hamano [this message]
2023-05-17 18:47     ` Victoria Dye
2023-05-18 15:44 ` [PATCH v2] " Shuqi Liang
2023-05-22 19:07   ` Victoria Dye
2023-05-23  4:38     ` 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=xmqqsfbvswci.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=cheskaqiqi@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=vdye@github.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.