Shane F. Carr

Results 2035 comments of Shane F. Carr

Flagging @justingrant for an API docs review. See: - https://icu4x-pr-artifacts.storage.googleapis.com/gha/4e9d5ee86fb558693589ab0403898b2ac9cb1099/rustdoc/icu_timezone/struct.TimeZoneIdMapper.html - https://icu4x-pr-artifacts.storage.googleapis.com/gha/4e9d5ee86fb558693589ab0403898b2ac9cb1099/rustdoc/icu_timezone/struct.TimeZoneIdMapperBorrowed.html - https://icu4x-pr-artifacts.storage.googleapis.com/gha/4e9d5ee86fb558693589ab0403898b2ac9cb1099/rustdoc/icu_timezone/struct.TimeZoneIdMapperWithFastCanonicalizationBorrowed.html

> One more thought: the index of a time zone id is

I think I addressed all of @justingrant's suggestions.

@justingrant says I can merge this PR since I have integrated his feedback.

@echeran, this has already been approved as you can see above. I merged main and ran datagen. I need another approval in order to merge.

The most modular setup would probably be 1. `icu_datagen_transform` defines DatagenProvider 2. `icu_datagen_driver` defines DatagenDriver 3. `icu_datagen` defines the `icu4x-datagen` binary

ICU4X-WG discussion: - @sffc - Main advantage, besides modularity, is reducing dependencies. The registry needs to depend on all of icu4x though. Can we put the registry in the metacrate?...

@eggrobin says he is not the correct person to review this from the code side. Adding back @echeran for the code review.

Two main reasons for the abstraction: 1. We can use serialize_bytes 2. We can serialize as a string for readability in JSON