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

Update Rust model with list of time zones #150

Open
richfab opened this issue Dec 2, 2024 · 4 comments
Open

Update Rust model with list of time zones #150

richfab opened this issue Dec 2, 2024 · 4 comments
Assignees

Comments

@richfab
Copy link
Contributor

richfab commented Dec 2, 2024

Follow-up of #146

Context

The time zones in the schemas were replaced with the content of the TZ identifier column from this list.

Problem

The Rust models are inconsistent with the spec because they use the previous list of time zones.

Solution

Update the Rust models with the list of time zones from the schemas.

@Alessandro100 Alessandro100 self-assigned this Dec 5, 2024
@Alessandro100
Copy link
Contributor

Looking at the current files, timezone is defined as
pub type Timezone = String;
Which has a comment mentioning that the values should be according to the link https://en.wikipedia.org/wiki/List_of_tz_zones. This link redirects to the correct list of new Timezones

Unless we want to change the data type from String to something more complex, there is nothing to be done here and can close the ticket

@richfab
Copy link
Contributor Author

richfab commented Dec 6, 2024

Got it, thank you for looking into it @Alessandro100.

In theory, the model should match the spec and only allow values ​​from the list https://en.wikipedia.org/wiki/List_of_tz_zones.

What do you think @tdelmas as the author and user of this model?

@tdelmas
Copy link
Collaborator

tdelmas commented Dec 6, 2024

As they should use the current valid timezone, I think updating the list is the correct way.

@richfab
Copy link
Contributor Author

richfab commented Dec 6, 2024

Thank you @tdelmas!

@Alessandro100 If it's a quick thing to do, please add the allowed values ​​from the list https://en.wikipedia.org/wiki/List_of_tz_zones.

If it's not quick to do, then please reassess the priority of this issue with @emmambd and @davidgamez. Thank you!

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

No branches or pull requests

3 participants