Skip to content

Conversation

@gnprice
Copy link
Member

@gnprice gnprice commented Dec 22, 2025

This is intended to check if the pattern failed to match, by checking whether the array parsed has zero elements. To do this, it looks at the length of the array... but then it was checking if that length, as a string, is empty. Oops. Instead, check if the length, as a number, is zero.

@gnprice gnprice added the maintainer review PR ready for review by Zulip maintainers label Dec 22, 2025
@gnprice
Copy link
Member Author

gnprice commented Dec 22, 2025

(Ran across this when I was experimenting this morning toward #2050: I'd edited pubspec.yaml by hand and didn't bother to put a proper commit-ID comment there, and the tools/check script aborted a few lines below this because ${parsed[0]} was unset, rather than properly fail the suite and proceed.)

Copy link
Member

@rajveermalviya rajveermalviya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice find @gnprice! LGTM, feel free to merge.

@chrisbobbe chrisbobbe added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Dec 23, 2025
This is intended to check if the pattern failed to match, by
checking whether the array `parsed` has zero elements.  To do this,
it looks at the length of the array... but then it was checking if
that length, as a string, is empty.  Oops.  Instead, check if the
length, as a number, is zero.
@gnprice gnprice merged commit ab1bb45 into zulip:main Dec 27, 2025
1 check passed
@gnprice gnprice deleted the pr-check-version branch January 2, 2026 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

integration review Added by maintainers when PR may be ready for integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants