Re: [PATCH v3 3/4] fetch-pack: expose fsckObjects configuration logic

From: Patrick Steinhardt <ps@pks.im>
Date: May 29, 2024, 05:52 UTC
Message-ID: <ZlbCiVsIwmqEFESo@tanuki>
In-Reply-To: <xmqqikyx6ew9.fsf@gitster.g>

On Tue, May 28, 2024 at 10:10:46AM -0700, Junio C Hamano wrote:

Show 31 quoted lines

> Patrick Steinhardt <ps@pks.im> writes:
> 
> >> +int fetch_pack_fsck_objects(void)
> >> +{
> >> +	fetch_pack_setup();
> >> +
> >> +	return fetch_fsck_objects >= 0
> >> +	       ? fetch_fsck_objects
> >> +	       : transfer_fsck_objects >= 0
> >> +	       ? transfer_fsck_objects
> >> +	       : 0;
> >> +}
> >
> > ... can we maybe rewrite it to something more customary here? The
> > following is way easier to read, at least for me.
> >
> > 	int fetch_pack_fsck_objects(void)
> > 	{
> > 		fetch_pack_setup();
> > 		if (fetch_fsck_objects >= 0 ||
> > 		    transfer_fsck_objects >= 0)
> > 			return 1;
> > 		return 0;
> > 	}
> 
> But do they mean the same thing?  In a repository where
> 
>     [fetch] fsckobjects = no
> 
> is set, no matter what transfer.fsckobjects says (or left unspecified),
> we want to return "no, we are not doing fsck".

Oh, of course they don't. This here would be a faithful conversion:

	int fetch_pack_fsck_objects(void)
	{
		fetch_pack_setup();
		if (fetch_fsck_objects >= 0)
			return fetch_fsck_objects;
		if (transfer_fsck_objects >= 0)
			return transfer_fsck_objects;
		return 0;
	}

Still easier to read in my opinion.

Patrick