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

Datagen: Consume CLDR-JSON resources keyed with default script #3772

Merged
merged 24 commits into from
Aug 4, 2023

Conversation

sffc
Copy link
Member

@sffc sffc commented Aug 3, 2023

Part of #2683
Depends on #3769
Depends on #3768

@sffc
Copy link
Member Author

sffc commented Aug 3, 2023

Commits through f084546 are #3768, and the rest is unique to this PR, except for two commits fixing a bug in LocaleExpander which I pulled out into #3769.

@sffc sffc removed request for a team, dminor, zbraniecki and Manishearth August 3, 2023 07:23
provider/datagen/src/transform/cldr/source.rs Outdated Show resolved Hide resolved
provider/datagen/src/source.rs Outdated Show resolved Hide resolved
@dpulls
Copy link

dpulls bot commented Aug 3, 2023

🎉 All dependencies have been resolved !

@@ -75,6 +75,7 @@ itertools = "0.10"
lazy_static = "1"
log = "0.4"
memchr = "2.5.0"
once_cell = "1"
Copy link
Member Author

Choose a reason for hiding this comment

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

@Manishearth to approve the new dependency. (We now have multiple places where it is useful)

Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to not have two lazy initialization dependencies, we should either use lazy_static everywhere or once_cell everywhere.

Slight preference for the latter since 1.70 has it in std

Copy link
Member

Choose a reason for hiding this comment

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

Approved with #3777 being 1.3 blocking (I'm fine with this landing first)

@sffc
Copy link
Member Author

sffc commented Aug 3, 2023

@robertbastian's changes LGTM. Looks like we need to add once_cell to the deps checker, and then we can merge?

@sffc
Copy link
Member Author

sffc commented Aug 3, 2023

Blocked on #3777

robertbastian
robertbastian previously approved these changes Aug 3, 2023
@sffc sffc merged commit 69f854e into unicode-org:main Aug 4, 2023
26 checks passed
@sffc sffc deleted the zhHansSG branch August 4, 2023 06:58
@sffc
Copy link
Member Author

sffc commented Aug 4, 2023

I'm admin-merging based on the approval from robertbastian, the clean CI, the fact that I had no substantive changes since that approval, and because I want to fix #2683 tonight.

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.

None yet

3 participants