Re: [PATCH 0/1] commit-graph: drop top-level --[no-]progress

From: Taylor Blau <me@ttaylorr.com>
Date: Sep 21, 2021, 20:38 UTC
Message-ID: <YUpC0moHj4K53Wk2@nand.local>
In-Reply-To: <87zgs593ja.fsf@evledraar.gmail.com>

On Tue, Sep 21, 2021 at 08:19:47PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 36 quoted lines

>
> On Mon, Sep 20 2021, Taylor Blau wrote:
>
> > On Mon, Sep 20, 2021 at 02:24:04PM -0700, Junio C Hamano wrote:
> >> Taylor Blau <me@ttaylorr.com> writes:
> >>
> >> > An open question is whether the same should be done for the multi-pack-index
> >> > command, whose top-level support for `--[no-]progress` was released in v2.32.0
> >> > with 60ca94769c (builtin/multi-pack-index.c: split sub-commands, 2021-03-30).
> >>
> >> We do not mind too much about "breaking backward compatibility" by
> >> removing the mistaken "git multi-pack-index --progress cmd", I would
> >> say.  It's not like people would type it once every day and removing
> >> the "support" will break their finger-memory.
> >
> > OK; if we don't mind then we could do something like the following on
> > top. But if we're OK to remove the top-level `--progress` option from
> > the commit-graph and multi-pack-index builtins at any time, then both
> > patches become far less urgent.
>
> I think just taking both this and your commit-graph patches is the right
> thing to do at this point. I.e. we almost entirely take:
>
>     git [git-opts] <cmd> [cmd-opts]
>
> Or:
>
>     git [git-opts] <cmd> <subcmd> [subcmd-opts]
>
> And almost never:
>
>     git [git-opts] <cmd> [global-subcmd-opts] <subcmd> [subcmd-opts]
>
> A notable exception is the --object-dir (I think I found out from
> Derrick at some point why that was even needed v.s. the top-level
> --git-dir, but I can't remember).

There's a good explanation in:

    https://lore.kernel.org/git/22366f81-65a6-55d1-706c-59f877127be0@gmail.com/

and a lot of related discussion happening throughout that whole sub-thread. The gist is that it's to be able to treat directories that look like they are a repository's object store (but don't actually belong to any real repository) as if they are an alternate.

> But just as a *general* comment on where our UI should and shouldn't be
> headed, I find your [1] an entirely unconvincing reply to [2]. I.e.:

I think that's a fine argument in the other direction. But to be fair, I consider the top-level '-c foo.bar=baz' to be different than a sub-command of the `commit-graph` builtin supporting `--progress`.

Perhaps you consider these the same, and I could even understand why. But to me, at least, I would be disappointed if we introduced a new sub-command of commit-graph which didn't generate a progress meter, while still accepting `--progress`.

In other words, as a user, I would be somewhat confused if I didn't know any better to have asked for `--progress` in a mode which no progress will be generated. I imagine it would be confusing not to see any output *and* not have `--progress` be rejected as an unrecognized option.

Anyway. To be honest, I find this whole discussion a little too theoretical for my taste. I think the patch(es) that I wrote for commit-graph and multi-pack-index seem relatively uncontroversial, and (at least in the commit-graph case) fix a real problem that we could avoid leaking out into a release.

Thanks, Taylor