From: Junio C Hamano <gitster@pobox.com>
Date: Fri, 23 Feb 2024 18:41:41 GMT
Subject: Re: [PATCH v5 0/5] for-each-ref: add '--include-root-refs' option
Message-ID: <xmqqzfvrrpju.fsf@gitster.g>
In-Reply-To: <20240223100112.44127-1-karthik.188@gmail.com>

Karthik Nayak <karthik.188@gmail.com> writes:

> Changes from v4:
> 1. Fixed erratic whitespace
> 2. Remove braces from single line block
> 3. Starting the comments with a capital and also adding more context.
> 4. Removed a duplicate check.

Does #4 refer to this removal?

 	if (filter->kind == FILTER_REFS_KIND_MASK && kind == FILTER_REFS_DETACHED_HEAD)
 		kind = FILTER_REFS_PSEUDOREFS;
 	else if (!(kind & filter->kind))
 		return NULL;
 
-	if (!(kind & filter->kind))
-		return NULL;
-
 	if (!filter_pattern_match(filter, refname))
 		return NULL;
 

If filter->kind is MASK and kind is set to filter detached HEAD, we
assign to and change the value of kind to FILTER_REFS_PSEUDOREFS,
so it is unclear if the freestanding "if kind and filter->kind does
not overlap, return NULL" was redundant or outright buggy.

The hunk just stood out to me, but I haven't read other parts of the
series yet.

Thanks.