Skip to content

Parse 'approach NACK' as AckType::ApproachNack#71

Merged
maflcko merged 1 commit intomaflcko:mainfrom
l0rinc:l0rinc/summary-approach-nack-case-insensitive
Jan 5, 2026
Merged

Parse 'approach NACK' as AckType::ApproachNack#71
maflcko merged 1 commit intomaflcko:mainfrom
l0rinc:l0rinc/summary-approach-nack-case-insensitive

Conversation

@l0rinc
Copy link
Contributor

@l0rinc l0rinc commented Jan 2, 2026

In bitcoin/bitcoin#34165 (review), the text Concept ACK, but approach NACK was summarized as Concept NACK by DrahtBot because it only matched Approach NACK with the exact capitalization.

Make the [Aa]pproach/[Cc]oncept prefix capitalization-insensitive and add a regression test.

@DrahtBot
Copy link
Collaborator

DrahtBot commented Jan 2, 2026

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

The summary comment parser treated "Concept ACK, but approach NACK." as a `Concept NACK` because it only matched "Approach NACK" with the exact capitalization.

Make the "Approach/Concept N?ACK" pattern capitalization-insensitive and add regression tests.

Ref: bitcoin/bitcoin#34165 (review)
@l0rinc l0rinc force-pushed the l0rinc/summary-approach-nack-case-insensitive branch from d3a7723 to 15c56be Compare January 4, 2026 22:47
@maflcko maflcko merged commit 15c56be into maflcko:main Jan 5, 2026
1 check passed
@maflcko
Copy link
Owner

maflcko commented Jan 5, 2026

thx

@l0rinc l0rinc deleted the l0rinc/summary-approach-nack-case-insensitive branch January 5, 2026 17:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments