Re: [PATCH 2/8] builtin/reflog: improve grouping of subcommands

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 24, 2025, 16:45 UTC
Message-ID: <xmqqikjhilm3.fsf@gitster.g>
In-Reply-To: <aIHj4NWr3M5lXLtD@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 21 quoted lines

> On Wed, Jul 23, 2025 at 01:14:19PM -0500, Justin Tobler wrote:
>> > @@ -404,10 +404,10 @@ int cmd_reflog(int argc,
>> >  	struct option options[] = {
>> >  		OPT_SUBCOMMAND("show", &fn, cmd_reflog_show),
>> >  		OPT_SUBCOMMAND("list", &fn, cmd_reflog_list),
>> > -		OPT_SUBCOMMAND("expire", &fn, cmd_reflog_expire),
>> > -		OPT_SUBCOMMAND("delete", &fn, cmd_reflog_delete),
>> >  		OPT_SUBCOMMAND("exists", &fn, cmd_reflog_exists),
>> > +		OPT_SUBCOMMAND("delete", &fn, cmd_reflog_delete),
>> >  		OPT_SUBCOMMAND("drop", &fn, cmd_reflog_drop),
>> > +		OPT_SUBCOMMAND("expire", &fn, cmd_reflog_expire),
>> >  		OPT_END()
>> >  	};
>> 
>> Structing the subcommands order in such a manner seems sensible, but I'm
>> not sure the pattern will be recognized by others that may add
>> subcommands in the future. Maybe we could leave a comment that mentions
>> the order?
>
> Hm, dunno. I feel like it's subjective where to add a command anyway, so
> I'm not sure that a comment would be allt hat helpful.

I'd agree on both counts. The only pattern I can see myself is to have read-only operations first and then read-write operations, but even there, the choice of "is it read-only?" as an axis feels very much arbitrary (another obvious one is to list from the everyday use to less often used to finally only administrative ones).

If the read-write operations are ordered by severity, I would place expire (affects only stale entries) between delete (affects one entry in a reflog) and drop (deletes the whole thing). But that again is fairly arbitrary.