Re: [PATCH v2] reflog: implement subcommand to drop reflogs

From: Patrick Steinhardt <ps@pks.im>
Date: Mar 13, 2025, 14:45 UTC
Message-ID: <Z9Lvc2YKr5jD5D6h@pks.im>
In-Reply-To: <CAOLa=ZQ3Rgi+yyma0Zo6uK04eu9Lk1dk_uYTU-4hFfh9POTNYA@mail.gmail.com>

On Thu, Mar 13, 2025 at 09:24:16AM -0500, Karthik Nayak wrote:

Show 16 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> > On Mon, Mar 10, 2025 at 01:36:25PM +0100, Karthik Nayak wrote:
> >> +			continue;
> >> +		}
> >> +
> >> +		ret |= refs_delete_reflog(get_main_ref_store(repo), ref);
> >> +		free(ref);
> >> +	}
> >
> > The code is correct, but do we want to maybe wrap this loop in the
> > `else` branch to guide the reader and make it blindingly obvious that
> > the loop does nothing `if (do_all)`?
> >
> 
> Wouldn't it be simpler to return at the end of the `if (do_all)`? I've
> added that, but if feel strongly about this form, happy to change it.

I don't feel overly strong about it. I'm not a huge fan of early returns as I think it's easier to reason about functions if they have a common exit path. But I can live with whatever solution you come up with.

Patrick