Re: [PATCH 1/2] doc: update the documentation of pack-objects and repack

From: Damien Robert <damien.olivier.robert@gmail.com>
Date: Mar 3, 2020, 21:23 UTC
Message-ID: <20200303212302.znhumbjo7lywyhvh@doriath>
In-Reply-To: <xmqqa74x8e9k.fsf@gitster-ct.c.googlers.com>

From Junio C Hamano, Tue 03 Mar 2020 at 10:49:43 (-0800) :

Show 13 quoted lines

> Damien Robert <damien.olivier.robert@gmail.com> writes:
> 
> > The problem here is that
> > `--keep-true-parents`, `--no-empty`, `--all`, `--reflog`, `--indexed-objects`,
> > `--exclude-promisor-objects`
> > are always passed and not driven by any options of `git repack`, so I
> > did not know where else to put them.
> 
> Ah, I think I may have misread the patch, then.  Why do readers who
> wanted to learn 'git repack' even need to see what the command does
> under the hood, driving what other low-level commands by passing
> what options, in the first place?  Such implementation details can
> change without affecting end-users, no?

So do you suggest instead to remove all references to 'git-pack-objects' in 'git-repack'? As I explained in my previous email, if some options reference pack-objects, I think they should all do for consistency.

I also think that the situation of git-repack is a bit special: first it is a very thin wrapper around git-pack-objects, so the implementation details are not really abstracted from git-pack-objects. Furthermore it is at an intermediate 'level' between a high level command like 'git gc' and a plumbing command like 'git pack-objects'. So the user interested in 'git repack' is probably interested in some low level details.

Now to give an exemple, the doc of git-repack states:
    This command is used to combine all objects that do not currently
    reside in a "pack", into a pack.
This is a high level overview, but the user who knows a bit about Git
internals may wonder what exactly 'all' entails: non local objects, kept
objects, promisor objects, unreachable objects?
Knowing that the default options passed are:
`--keep-true-parents`, `--all`, `--reflog`, `--indexed-objects`, `--exclude-promisor-objects`
answers this questions: it is essentially all objects except unreachable ones.
Here I think that these technical details are more precise that whatever
sentence I could come up with, but I am happy to hear suggestions :)

I agree however that `--non-empty` is an implementation detail.