git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Ben Knoble <ben.knoble@gmail.com>
Cc: git@vger.kernel.org, Yonatan Roth <yroth@paloaltonetworks.com>,
	david asraf <dasraf9@gmail.com>
Subject: Re: [PATCH 08/11] builtin/maintenance: let tasks do maintenance before and after detach
Date: Fri, 30 May 2025 16:05:25 +0200	[thread overview]
Message-ID: <aDm7JeVg9vqsq-09@pks.im> (raw)
In-Reply-To: <6E33013D-8336-4BE1-9DBB-CAFF835A49C3@gmail.com>

On Fri, May 30, 2025 at 08:56:38AM -0400, Ben Knoble wrote:
> > @@ -1599,20 +1600,25 @@ static const struct maintenance_task tasks[] = {
> > static int maybe_run_task(const struct maintenance_task *task,
> >             struct repository *repo,
> >             struct maintenance_run_opts *opts,
> > -              struct gc_config *cfg)
> > +              struct gc_config *cfg,
> > +              int before)
> 
> Perhaps we can use a small enum…

> > @@ -1641,6 +1647,10 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
> >   }
> >   free(lock_path);
> > 
> > +    for (size_t i = 0; i < opts->tasks_nr; i++)
> > +        if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, 1))
> 
> So that is clear what « 1 » (« BEFORE ») does here…
> 
> > +            result = 1;
> > +
> >   /* Failure to daemonize is ok, we'll continue in foreground. */
> >   if (opts->detach > 0) {
> >       trace2_region_enter("maintenance", "detach", the_repository);
> > @@ -1649,7 +1659,7 @@ static int maintenance_run_tasks(struct maintenance_run_opts *opts,
> >   }
> > 
> >   for (size_t i = 0; i < opts->tasks_nr; i++)
> > -        if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg))
> > +        if (maybe_run_task(&tasks[opts->tasks[i]], r, opts, cfg, 0))
> 
> And « 0 » (« AFTER ») does here?

Yeah, I was very close to adding one, but then decided against it to not
produce additional churn. But agreed, it would be way easier to read
this way, so let me add one.

Patrick

  reply	other threads:[~2025-05-30 14:05 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-05-30 12:56 [PATCH 08/11] builtin/maintenance: let tasks do maintenance before and after detach Ben Knoble
2025-05-30 14:05 ` Patrick Steinhardt [this message]
  -- strict thread matches above, loose matches on Subject: below --
2025-05-27 14:04 [PATCH 00/11] builtin/maintenance: fix ref lock races when detaching Patrick Steinhardt
2025-05-27 14:04 ` [PATCH 08/11] builtin/maintenance: let tasks do maintenance before and after detach Patrick Steinhardt
2025-05-27 17:01   ` Ramsay Jones
2025-05-28  7:02     ` Patrick Steinhardt

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=aDm7JeVg9vqsq-09@pks.im \
    --to=ps@pks.im \
    --cc=ben.knoble@gmail.com \
    --cc=dasraf9@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=yroth@paloaltonetworks.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).