git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Karthik Nayak <karthik.188@gmail.com>
Cc: Junio C Hamano <gitster@pobox.com>, git@vger.kernel.org
Subject: Re: [PATCH 9/9] refs: add a '--required' flag to 'git refs optimize'
Date: Wed, 15 Oct 2025 14:14:26 +0200	[thread overview]
Message-ID: <aO-QIn7KAVlBmfRq@pks.im> (raw)
In-Reply-To: <CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com>

On Wed, Oct 15, 2025 at 02:29:35AM -0700, Karthik Nayak wrote:
> Karthik Nayak <karthik.188@gmail.com> writes:
> 
> > Junio C Hamano <gitster@pobox.com> writes:
> >
> >> Karthik Nayak <karthik.188@gmail.com> writes:
> >>
> >>> Junio C Hamano <gitster@pobox.com> writes:
> >>>
> >>>> Junio C Hamano <gitster@pobox.com> writes:
> >>>>
> >>>>> Perhaps "--check-" followed by a word specific to what we are trying
> >>>>> to achieve (e.g., if we are trying to see if auto-compaction is
> >>>>> necessary, "--check-for-auto" "check for auto compaction")?  I
> >>>>> dunno.
> >>>>
> >>>> After reading what you did in the previou step, I am reasonably sure
> >>>> "required" is a wrong word to use, with or without other words like
> >>>> "check".  Semantically it is similar to the should_pack_refs() check
> >>>> that we use for pack-refs even before "optimize" came.  We expect it
> >>>> to answer this question cheaply: are we better off if we repacked,
> >>>> or can we go on without repacking for now?  It is not about "are we
> >>>> performing so poorly that we MUST optimize now?"
> >>>
> >>> I agree '--required' isn't the best name, and like we discussed
> >>> '--dry-run' wouldn't be either since that would imply that the work is
> >>> being done but not persisted.
> >>>
> >>> I was leaning towards '--check', which is simple. But It might be nicer
> >>> to be verbose here and simply add something like '--is-worthwhile'.
> >>>
> >>> Being verbose here is okay, since it will only be used sparingly and
> >>> specifically by those who require such a use case.
> >>
> >> Nah, "worthwhile" is relative and it would be less meaningful
> >> without expressing for what goal we are judging how it is worthwhile
> >> to do.
> >>
> >
> > I see what you mean.
> >
> >> Choosing a phrase around "check" is better, I would think.
> >
> > How about simply `--check`. Since the flag can be used with other
> > existing flags, it would make more sense to do
> >
> >   git refs optimize --all --auto --check
> >   git refs optimize --check
> >   git refs optimize --all --check
> >
> > I will send in a new version around this and we can discuss further on
> > top of that!
> 
> After speaking to Patrick today about this and his work on 'git
> maintenance', we realized we can actually broaden the scope. We could
> implement something like 'git maintenance needed'. This would check on a
> whole if repository maintenance is needed for refs and objects.

Yup. The idea here is that git-maintenance(1) is already split up into
different tasks, one of which is the "pack-refs" task. And each of these
tasks already has a callback to verify whether or not we need to execute
the task.

So if we build on top of that mechanism we can easily broaden the scope
of this patch series to:

  - Check whether maintenance is needed for any of the enabled tasks.

  - Check whether maintenance is needed for a single task.

  - Check whether maintenance is needed given a specific maintenance
    strategy.

Another benefit is that it's a bit easier to put a verb into a proper
subcommand of git-maintenance(1) instead of having to add a verb-ish
flag to git-pack-refs(1). `git maintenance is-needed` feels way more
natural to me at least compared to `git refs optimize --check`.

Thanks!

Patrick

  reply	other threads:[~2025-10-15 12:14 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-10 10:27 [PATCH 0/9] refs: add a '--required' flag to 'git refs optimize' Karthik Nayak
2025-10-10 10:27 ` [PATCH 1/9] refs: move to using the '.optimize' functions Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13  8:18     ` Karthik Nayak
2025-10-10 10:27 ` [PATCH 2/9] refs: cleanup code around optimization Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13  8:22     ` Karthik Nayak
2025-10-10 10:27 ` [PATCH 3/9] refs: rename 'pack_refs_opts' to 'optimize_refs_opts' Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13  8:52     ` Karthik Nayak
2025-10-10 10:27 ` [PATCH 4/9] t/pack-refs-tests: move the 'test_done' to callees Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13  8:54     ` Karthik Nayak
2025-10-10 10:27 ` [PATCH 5/9] t/t0450: split whitespace consistency check per subcommand Karthik Nayak
2025-10-10 10:27 ` [PATCH 6/9] reftable/stack: return stack segments directly Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13  9:01     ` Karthik Nayak
2025-10-13 11:10       ` Patrick Steinhardt
2025-10-10 10:27 ` [PATCH 7/9] reftable/stack: add function to check if optimization is required Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13  9:04     ` Karthik Nayak
2025-10-10 10:27 ` [PATCH 8/9] refs: add a `optimize_required` field to `struct ref_storage_be` Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13  9:46     ` Karthik Nayak
2025-10-10 10:27 ` [PATCH 9/9] refs: add a '--required' flag to 'git refs optimize' Karthik Nayak
2025-10-10 11:22   ` Patrick Steinhardt
2025-10-13 12:37     ` Karthik Nayak
2025-10-13 13:40     ` Junio C Hamano
2025-10-13 14:37       ` Junio C Hamano
2025-10-14 15:08         ` Karthik Nayak
2025-10-14 17:46           ` Junio C Hamano
2025-10-15  7:50             ` Srivastava, Nitin
2025-10-15  8:19             ` Karthik Nayak
2025-10-15  9:29               ` Karthik Nayak
2025-10-15 12:14                 ` Patrick Steinhardt [this message]
2025-10-15 20:17                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aO-QIn7KAVlBmfRq@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).