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

Fix invalid IANA time zone identifier for Atlantic/Cape_Verde #2927

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

andrelaszlo
Copy link
Contributor

@andrelaszlo andrelaszlo commented Apr 4, 2024

Motivation / Background

Fixes: #2928

This Pull Request has been created because our tests using Faker time zones mostly pass, but fails when the Atlantic/Cabo_Verde time zone is returned. We validate that time zones are valid IANA identifiers, and Atlantic/Cabo_Verde is called Atlantic/Cape_Verde in the Time Zone Database.

Additional information

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug, refactor something, or add a feature.
  • Tests and Rubocop are passing before submitting your proposed changes.

@andrelaszlo
Copy link
Contributor Author

andrelaszlo commented Apr 4, 2024

Unrelated, but I'm not sure it makes sense to translate the time zone names at all, since they're identifiers. Here's an example:

      time_zone: [África/Argel, África/Cairo, África/Casablanca, África/Harare, ...]

/faker/lib/locales/es-MX.yml:15

Each timezone has a name that uniquely identifies the timezone. Inexperienced users are not expected to select these names unaided. Distributors should provide documentation and/or a simple selection interface that explains each name via a map or via descriptive text like "Czech Republic" instead of the timezone name "Europe/Prague". If geolocation information is available, a selection interface can locate the user on a timezone map or prioritize names that are geographically close. For an example selection interface, see the tzselect program in the tz code. The Unicode Common Locale Data Repository contains data that may be useful for other selection interfaces; it maps timezone names like Europe/Prague to locale-dependent strings like "Prague", "Praha", "Прага", and "布拉格".

This quote from Theory and pragmatics of the tz code and data - Timezone identifiers hints that the timezone names can be mapped to translations, but the unique identifier aspect gets lost if the timezone's name itself is translated.

@thdaraujo thdaraujo force-pushed the fix_invalid_iana_tz_identifier branch from c9c1de0 to 678f0d3 Compare April 19, 2024 15:40
Copy link
Contributor

@thdaraujo thdaraujo left a comment

Choose a reason for hiding this comment

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

excellent, thanks for adding tests!

@thdaraujo thdaraujo changed the title Fix invalid IANA time zone identifier Fix invalid IANA time zone identifier for Atlantic/Cape_Verde Apr 19, 2024
@thdaraujo thdaraujo merged commit 8f01a57 into faker-ruby:main Apr 19, 2024
8 checks passed
@andrelaszlo andrelaszlo deleted the fix_invalid_iana_tz_identifier branch April 19, 2024 16:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid Timezone: Atlantic/Cabo_Verde
3 participants