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

Add support for Etc/UTC time zone identifier without tzdb #14185

Merged
merged 8 commits into from Jan 12, 2024

Conversation

femto
Copy link
Contributor

@femto femto commented Jan 8, 2024

refers to this #14074,

Also I took the liberty of adding a few common things to .gitignore,
I can seperate that into a distinguish PR if you want

Copy link
Member

@straight-shoota straight-shoota left a comment

Choose a reason for hiding this comment

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

Please seperate unrelated changes into independent PRs. The gitignore changes are a different issue.

The time location change requires updates to the API docs. At least on the directly affected methods. Possibly elsewhere like the class documentation.

.gitignore Outdated Show resolved Hide resolved
@femto femto force-pushed the feature/etc_utc branch 2 times, most recently from a5637bf to fb7bdb6 Compare January 8, 2024 11:54
@femto
Copy link
Contributor Author

femto commented Jan 8, 2024

The time location change requires updates to the API docs. At least on the directly affected methods. Possibly elsewhere like the class documentation.

what do you mean by api docs? the comment before the methods? changed

@femto
Copy link
Contributor Author

femto commented Jan 8, 2024

ps: could #14174 be fixed?
it's just inconvient for make std_spec to fail on my machine :(

spec/std/time/location_spec.cr Outdated Show resolved Hide resolved
src/time/location.cr Show resolved Hide resolved
@straight-shoota
Copy link
Member

straight-shoota commented Jan 8, 2024

ps: could #14174 be fixed? it's just inconvient for make std_spec to fail on my machine :(

It's unclear what's even going on there, so I wouldn't know how it could possible be fixed. So far nobody has been able to reproduce this issue on their machine, so unless you can dig out more information, I fear everyone else is pretty much in the blind.

@straight-shoota straight-shoota changed the title Feature/etc_utc Add support for Etc/UTC time zone identifier without tzdb Jan 8, 2024
src/time/location.cr Outdated Show resolved Hide resolved
femto and others added 3 commits January 9, 2024 17:39
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
Co-authored-by: Johannes Müller <straightshoota@gmail.com>
Co-authored-by: George Dietrich <george@dietrich.app>
@femto
Copy link
Contributor Author

femto commented Jan 9, 2024

@straight-shoota changed, I'm currently reading the specification now,
so I still has few "why this" questions,
and also some "crystal internal implementation" questions,
could we setup a meeting to talk? I think that's more efficient.

Comment on lines 281 to 285
# `UTC` is a special identifier, empty string represents a fallback mechanism.
# `Etc/UTC` is technically a tzdb identifier which could potentially point to anything.
# But we map it to `Location::UTC` directly for convenience which allows it to work
# without a copy of the database.
UTC
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong indentation

@femto
Copy link
Contributor Author

femto commented Jan 9, 2024 via email

@straight-shoota straight-shoota added this to the 1.12.0 milestone Jan 9, 2024
@Blacksmoke16 Blacksmoke16 linked an issue Jan 9, 2024 that may be closed by this pull request
@straight-shoota straight-shoota merged commit 11b107b into crystal-lang:master Jan 12, 2024
56 checks passed
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.

Support Etc/UTC timezone location
5 participants