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: Refactor ResolveLocale to simplify calls #849

Merged
merged 4 commits into from
Feb 29, 2024

Conversation

gibson042
Copy link
Contributor

@gibson042 gibson042 commented Dec 15, 2023

This has been bugging me for quite a while.

  • Rename dataLocaleData aliases to the more accurate resolvedLocaleData
  • Update the return value to include locale data rather than its lookup key
  • Capitalize the return value static field names

@gibson042 gibson042 requested a review from ryzokuken December 15, 2023 20:35
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.

I am entirely for this change -- I've found the name _dataLocaleData_ to be borderline unreadable -- though I'm concerned that others might dislike losing the relationship between the name of the alias and the name of the [[DataLocale]] slot. @ryzokuken @sffc @FrankYFTang

@sffc
Copy link
Contributor

sffc commented Jan 17, 2024

The word "resolved" to me means that the data went through some resolution algorithm, which I guess it did because of dataLocale. I think better would be to rename the variables to "resolvedLocale" and "resolvedLocaleData". But I don't have a strong preference.

@anba
Copy link
Contributor

anba commented Jan 18, 2024

I wonder if we could remove the term "dataLocale" from the spec. I've created #853 as a possible alternative to this PR.

@ryzokuken ryzokuken added editorial Involves an editorial fix needs review labels Feb 22, 2024
@gibson042 gibson042 changed the title Editorial: Rename _dataLocaleData_ aliases to the more accurate _resolvedLocaleData_ Editorial: Simplify ResolveLocale calls Feb 27, 2024
@gibson042 gibson042 force-pushed the 2023-12-resolvedlocaledata-alias branch from f669ae0 to 3a03129 Compare February 27, 2024 17:40
@gibson042
Copy link
Contributor Author

@ben-allen @ryzokuken This has extended to change the shape of the ResolveLocale return value and refactor an internal slot of NumberFormat and RelativeTimeFormat instances, so I'm requesting re-review before merging.

@gibson042 gibson042 changed the title Editorial: Simplify ResolveLocale calls Editorial: Refactor ResolveLocale to simplify calls Feb 29, 2024
@gibson042 gibson042 merged commit 9c7c1b9 into tc39:main Feb 29, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial Involves an editorial fix needs review
Projects
Status: Previously Discussed
Development

Successfully merging this pull request may close these issues.

5 participants