On Sat, Dec 28, 2024 at 9:35 PM Junio C Hamano <gitster@xxxxxxxxx> wrote: > > Shubham Kanodia <shubham.kanodia10@xxxxxxxxx> writes: > > >> 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. There wasn't a good reason for doing it that way. I guess I was trying to understand the second argument for `for_each_remote` would be best used if the command was called directly (while avoiding a compilation warning), but looking at a few other usages of `for_each_remote` I realised that it could just be marked unused in this case (since we aren't doing anything with it). I should've probably looked deeper and learnt from existing patterns (e.g. `maintenance_task_prefetch`) — which I have in my last patch. > >> 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. Exactly — the first failing remote will signal that the `prune-remote-refs` task has failed via an immediate `return 1`. The maintenance command uses this to register the exit code of the top level command to 1, while continuing to execute all other tasks anyway. Thanks, Shubham K
