Re: [PATCH] maintenance: add prune-remote-refs task

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 28, 2024, 16:05 UTC
Message-ID: <xmqqr05r4wu5.fsf@gitster.g>
In-Reply-To: <CAG=Um+0a+ugf+gWUDS3htj3u2tewzOrH+xGbF+2A+w4ofjQfKg@mail.gmail.com>

Shubham Kanodia <shubham.kanodia10@gmail.com> writes:

Show 5 quoted lines

>> Hmph, is there a reason why you need two loops, instead of
>> for-each-remote calling a function that does the run_command()
>> thing?
>
> It can be collapsed into one.

Sorry, but that is not an answer, as my question was not a suggestion to change anything.

It was a question asking you if there was a specific reason why the code was structured the way it was written. If there is another way to write it, you need to answer why the alternative wasn't picked.

Show 11 quoted lines

>> This loop does not stop at the first error, but returns a non-zero
>> error after noticing even a single remote fail to run prune, which
>> sounds like a seneible design.  Would an error percolate up the same
>> way when two different tasks run and one of them fails in the
>> control folow in "git maintenance"?  Just want to see if we are
>> being consistent with the surrounding code.
>
> Fair point. I'll make the process flow identical to the prefetch refs
> task that works similarly across remotes.
> It returns as soon as the first remote fails (without necessarily
> affecting other tasks).

... and the first failure signals the caller a failure? That would match what you did in your new feature, which is perfect.

Thanks.