-
Notifications
You must be signed in to change notification settings - Fork 21.4k
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
Rename Europe/Kiev to Europe/Kyiv in ActiveSupport::TimeZone #44273
Comments
I’d imagine it’s easier to add a timezone-alias, that way we avoid breaking change. |
I think Rails already have a mapping for Kyiv. Endusers can use either "Kyiv" or "Europe/Kiev" value. https://github.com/rails/rails/blob/main/activesupport/lib/active_support/values/time_zone.rb#L99 The problem goes down to the TZInfo library which is reliant on the IANA Time Zone Database. |
Yeah, as @siriniok notes, the only mention inside Rails is where we map our timezone name "Kyiv" to the underlying database's internal identifier We can't stop individual applications from choosing to show the internal value to users, of course (and depending on their circumstances, that may make sense: sometimes you want a user to choose a city from a list, but sometimes it's clearer to have the user choose an IANA Time Zone entry by its identifier). I don't believe we're doing anything to encourage broad use of the internal name. The above quote from the docs, mapped to this entry in our data, would be:
i.e., our API as described attempts to perform exactly the sort of mapping recommended in the "more context" block. Our codebase contains one instance of the string "Kiev", and it's in the linked line:
The only result of changing that would be to entirely break timezone handling for Ukraine-based users. |
Please reopen this issue.
|
@matthewd please reopen this issue. All points mentioned by @mr-mig are still valid. In addition current version of tzinfo library follows change made in tzdb 2022b. So...
...isn't true anymore. Also every Ukraine-based user would welcome this change even if it somehow temporary breaks their code. In this case spelling correctness is more important. In general keeping status quo here isn't a wise choice in long-term as timezone names aren't static and will keep changing. |
@matthewd, |
I wonder if we can reconsider this, as folks point out the newer tzdata gem links both timezones, so it seems like the compatibility concerns are resolved? See: tzinfo/tzinfo-data@5a58dcb I've definitely not investigated all of the potential breakage this might cause, but if Rails could essentially use either one and tzdata does the right thing we can change it, maybe. |
Ok, after looking into this some more, I think we cannot change this yet. While 2022 db was updated, it's still a bit too recent to break applications if the OS tzdata is not up to date. We would need a way to handle both values for existing data, perhaps relying on tzinfo to canonicalize. Since this qualifies as a feature request, I'm going to close it. |
I will take liberty and sidestep the bug report template.
I would like to reanimate the discussion about renaming
Europe/Kiev
toEurope/Kiyv
as a timezone representation in ActiveSupport::TimeZone.You can find prior closed PRs in Rails repo, here is the latest.
Problem Descriptions
There are a lot of people in the world who care about the correct English representation of
Kyiv
. See KyivNotKiev on Wikipedia or this youtuve video.When using Rails and showing time zone information for users (e.g. in timezone picker UI element), it is straightforward to use the data from
ActiveSupport::TimeZone
.The docs explicitly say that:
More context
The latest PR was rejected with the comments from @rafaelfranca stating that the data is taken from tzdata.
I want to emphasise this:
👉🏼 Tzdata values are not supposed to be presented to end users.
There are discussions on tz mailing list regarding the same topic. I want to emphasise the comment from Paul Eggert:
All of that lead me to think that it is the responsibility of Rails as an application framework that provides user-facing content to update the value accordingly.
How to proceed?
I would love to know what is needed and should be done to make this change happen?
The text was updated successfully, but these errors were encountered: