Skip to content

fix(targets): Target parser should accept both x and X#5894

Merged
asottile-sentry merged 2 commits into
getsentry:mainfrom
BYK:patch-1
Jul 9, 2025
Merged

fix(targets): Target parser should accept both x and X#5894
asottile-sentry merged 2 commits into
getsentry:mainfrom
BYK:patch-1

Conversation

@BYK

@BYK BYK commented Jul 9, 2025

Copy link
Copy Markdown
Member

Changes the targets parser to be case insensitive so we handle both x and X as checked-off.

Alternatively, we can make the char-class [ xX].

Alternatively we can make the entire regex case-insensitive.

This fixes the issue reported here: getsentry/craft#611 (comment) (second item)

Related failure: #5882

Changes the targets parser to be case insensitive so we handle both `x` and `X` as checked-off.

Alternatively, we can make the char-class `[ xX]`.

This fixes the issue reported here: getsentry/craft#611 (comment) (second item)
@BYK BYK requested a review from a team as a code owner July 9, 2025 20:59
@BYK

BYK commented Jul 9, 2025

Copy link
Copy Markdown
Member Author

cc @stephanie-anderson -- Not sure why your edit got in as a X instead of the regulat x. Did you click on the checkbox from the UI or manually edited the issue description?

@BYK

BYK commented Jul 9, 2025

Copy link
Copy Markdown
Member Author

FYI validated that this is the case by looking at the logs and testing the regexp: https://regex101.com/r/ivTbZM/1

@asottile-sentry

Copy link
Copy Markdown
Contributor

Alternatively, we can make the char-class [ xX].

let's do this just for specificity sake

@BYK

BYK commented Jul 9, 2025

Copy link
Copy Markdown
Member Author

@asottile-sentry done

@BYK

BYK commented Jul 9, 2025

Copy link
Copy Markdown
Member Author

We should probably retry the failing release right after merging this as a test (and just fix that half-release)

@asottile-sentry asottile-sentry left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@asottile-sentry asottile-sentry merged commit 14106b8 into getsentry:main Jul 9, 2025
3 checks passed
@BYK BYK deleted the patch-1 branch July 9, 2025 21:15
@romtsn

romtsn commented Jul 9, 2025

Copy link
Copy Markdown
Member

We should probably retry the failing release right after merging this as a test (and just fix that half-release)

#5882 success!

@kahest

kahest commented Jul 10, 2025

Copy link
Copy Markdown

thank you @BYK!

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.

4 participants