diff mbox series

doc/technical: remove outdated MIDX default note

Message ID 20210922221342.6867-1-e@80x24.org (mailing list archive)
State New, archived
Headers show
Series doc/technical: remove outdated MIDX default note | expand

Commit Message

Eric Wong Sept. 22, 2021, 10:13 p.m. UTC
Multi-pack-index files are used by default since
commit 18e449f86b74bab35b150549c8342d252fe7ae00, so the
outdated note was misleading.

Signed-off-by: Eric Wong <e@80x24.org>
---
 Documentation/technical/multi-pack-index.txt | 2 --
 1 file changed, 2 deletions(-)

Comments

Derrick Stolee Sept. 23, 2021, 1:47 p.m. UTC | #1
On 9/22/2021 6:13 PM, Eric Wong wrote:
> Multi-pack-index files are used by default since
> commit 18e449f86b74bab35b150549c8342d252fe7ae00, so the
> outdated note was misleading.

You are correct that the note is misleading, but it is still correct.

A user can _disable_ reading MIDX files by setting core.multiPackIndex
to false. If this was in the user-facing docs, not the technical docs,
then I might push back on trying to make that distinction.

> -- The core.multiPackIndex config setting must be on to consume MIDX files.
> -

As it is, this deletion is fine.

A nit on your commit message: format commit references like this:

  18e449f86 (midx: enable core.multiPackIndex by default, 2020-09-25)

Thanks,
-Stolee
Taylor Blau Sept. 23, 2021, 5:45 p.m. UTC | #2
On Thu, Sep 23, 2021 at 09:47:03AM -0400, Derrick Stolee wrote:
> On 9/22/2021 6:13 PM, Eric Wong wrote:
> > Multi-pack-index files are used by default since
> > commit 18e449f86b74bab35b150549c8342d252fe7ae00, so the
> > outdated note was misleading.
>
> You are correct that the note is misleading, but it is still correct.
>
> A user can _disable_ reading MIDX files by setting core.multiPackIndex
> to false. If this was in the user-facing docs, not the technical docs,
> then I might push back on trying to make that distinction.

It may present a fuller picture to instead say:

  The core.multiPackIndex config setting must be on (which is the default)
  to consume MIDX files. Setting it to `false` prevents Git from reading a
  MIDX file, even if one exists.

Thanks,
Taylor
Junio C Hamano Sept. 23, 2021, 8:27 p.m. UTC | #3
Taylor Blau <me@ttaylorr.com> writes:

> On Thu, Sep 23, 2021 at 09:47:03AM -0400, Derrick Stolee wrote:
>> On 9/22/2021 6:13 PM, Eric Wong wrote:
>> > Multi-pack-index files are used by default since
>> > commit 18e449f86b74bab35b150549c8342d252fe7ae00, so the
>> > outdated note was misleading.
>>
>> You are correct that the note is misleading, but it is still correct.
>>
>> A user can _disable_ reading MIDX files by setting core.multiPackIndex
>> to false. If this was in the user-facing docs, not the technical docs,
>> then I might push back on trying to make that distinction.
>
> It may present a fuller picture to instead say:
>
>   The core.multiPackIndex config setting must be on (which is the default)
>   to consume MIDX files. Setting it to `false` prevents Git from reading a
>   MIDX file, even if one exists.

Do you want to replace the one-line sentence with that paragraph,
instead of removing it, in a replacement patch?
Taylor Blau Sept. 23, 2021, 8:32 p.m. UTC | #4
On Thu, Sep 23, 2021 at 01:27:31PM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > On Thu, Sep 23, 2021 at 09:47:03AM -0400, Derrick Stolee wrote:
> >> On 9/22/2021 6:13 PM, Eric Wong wrote:
> >> > Multi-pack-index files are used by default since
> >> > commit 18e449f86b74bab35b150549c8342d252fe7ae00, so the
> >> > outdated note was misleading.
> >>
> >> You are correct that the note is misleading, but it is still correct.
> >>
> >> A user can _disable_ reading MIDX files by setting core.multiPackIndex
> >> to false. If this was in the user-facing docs, not the technical docs,
> >> then I might push back on trying to make that distinction.
> >
> > It may present a fuller picture to instead say:
> >
> >   The core.multiPackIndex config setting must be on (which is the default)
> >   to consume MIDX files. Setting it to `false` prevents Git from reading a
> >   MIDX file, even if one exists.
>
> Do you want to replace the one-line sentence with that paragraph,
> instead of removing it, in a replacement patch?

That's what I was hoping for, and Eric did that here:

    https://lore.kernel.org/git/20210923201252.GA25980@dcvr/

Thanks,
Taylor
diff mbox series

Patch

diff --git a/Documentation/technical/multi-pack-index.txt b/Documentation/technical/multi-pack-index.txt
index 1a73c3ee20..2038fb7995 100644
--- a/Documentation/technical/multi-pack-index.txt
+++ b/Documentation/technical/multi-pack-index.txt
@@ -36,8 +36,6 @@  Design Details
   directory of an alternate. It refers only to packfiles in that
   same directory.
 
-- The core.multiPackIndex config setting must be on to consume MIDX files.
-
 - The file format includes parameters for the object ID hash
   function, so a future change of hash algorithm does not require
   a change in format.