On Tue, May 28, 2024 at 10:24:35AM -0700, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
> > Patrick Steinhardt <ps@xxxxxx> 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".
>
> The original before it was made into a helper function was written
> as a cascade of ?: operators, because it had to be a single
> expression. As the body of a helper function, we now can sprinkle
> multiple return statements in it. I think the way that is easiest
> to understand is
>
> /* the most specific, if specified */
> if (fetch_fsck_objects >= 0)
> return fetch_fsck_objects;
> /* the less specific, catch-all for both directions */
> if (transfer_fsck_objects >= 0)
> return transfer_fsck_objects;
> /* the fallback hardcoded default */
> return 0;
>
> without the /* comments */.
Ah, right, didn't see this mail. My revised version looks the same as
yours, except for the added comments.
Patrick
Attachment:
signature.asc
Description: PGP signature
