Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Editorial: Remove unnecessary lowercase conversion of key #848

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

FrankYFTang
Copy link
Contributor

@FrankYFTang FrankYFTang commented Dec 15, 2023

Replaced with assertion since all the key come from internal slots that are already all in lowercase ASCII.

This issue is mentioned by @anba in #846 (comment)

Replaced with assertion since all the key come from internal slots that are already all in lowercase ASCII.
Copy link
Contributor

@ben-allen ben-allen left a comment

Choose a reason for hiding this comment

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

LGTM. presumably no new non-lowercase keys will ever be introduced, given that doing so would go directly against BCP47 recommendations.

@ben-allen
Copy link
Contributor

@gibson042 any problems with merging this one?

Copy link
Member

@ryzokuken ryzokuken left a comment

Choose a reason for hiding this comment

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

LGTM. @anba could you take a quick look before we hit merge?

@anba
Copy link
Contributor

anba commented Feb 6, 2024

LGTM. @anba could you take a quick look before we hit merge?

Looks good to me, too.

@ryzokuken ryzokuken added the editorial Involves an editorial fix label Feb 22, 2024
@ryzokuken ryzokuken merged commit 06aac66 into main Feb 22, 2024
3 checks passed
@ryzokuken ryzokuken deleted the FrankYFTang-patch-2 branch February 22, 2024 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Involves an editorial fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants