Re: [PATCH 1/5] multi-pack-index: prepare for 'expire' verb

["SZEDER Gábor" <szeder.dev@gmail.com>]

On Mon, Dec 10, 2018 at 05:35:28PM -0800, Stefan Beller wrote:
> On Mon, Dec 10, 2018 at 10:06 AM Derrick Stolee via GitGitGadget
> <gitgitgadget@gmail.com> wrote:
> >
> > From: Derrick Stolee <dstolee@microsoft.com>
> >
> > The multi-pack-index tracks objects in a collection of pack-files.
> > Only one copy of each object is indexed, using the modified time
> > of the pack-files to determine tie-breakers. It is possible to
> > have a pack-file with no referenced objects because all objects
> > have a duplicate in a newer pack-file.
> >
> > Introduce a new 'expire' verb to the multi-pack-index builtin.
> > This verb will delete these unused pack-files and rewrite the
> > multi-pack-index to no longer refer to those files. More details
> > about the specifics will follow as the method is implemented.
> >
> > Add a test that verifies the 'expire' verb is correctly wired,
> > but will still be valid when the verb is implemented. Specifically,
> > create a set of packs that should all have referenced objects and
> > should not be removed during an 'expire' operation.
> >
> > Signed-off-by: Derrick Stolee <dstolee@microsoft.com>
> > ---
> >  Documentation/git-multi-pack-index.txt |  8 +++++
> >  builtin/multi-pack-index.c             |  4 ++-
> >  midx.c                                 |  5 +++
> >  midx.h                                 |  1 +
> >  t/t5319-multi-pack-index.sh            | 47 ++++++++++++++++++++++++++
> >  5 files changed, 64 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
> > index f7778a2c85..822d83c845 100644
> > --- a/Documentation/git-multi-pack-index.txt
> > +++ b/Documentation/git-multi-pack-index.txt
> > @@ -31,6 +31,14 @@ verify::
> >         When given as the verb, verify the contents of the MIDX file
> >         at `<dir>/packs/multi-pack-index`.
> >
> > +expire::
> > +       When given as the verb,
> 
> Can it be given in another way? Or rather "if the verb is expire",
> then ...
> (I just checked the current man page, and both write and verify use
> this pattern as well. I find it strange as this first part of the sentence
> conveys little information, but is repeated 3 times now (once for
> each verb)).
> 
> Maybe we can restructure the man page to have it more like
> 
>     The following verbs are available:
>     +write::
>     +    create a new MIDX file, writing to <dir>/packs/multi-pack-index.
>     +
>     +verify::
>     +    verify the contents ...

I think a s/verb/subcommand/ would help a lot, too, because that's
what we call it everywhere else.