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 chrono FixedOffset #156

Merged
merged 1 commit into from
Nov 14, 2023
Merged

Conversation

willtrnr
Copy link
Contributor

Adds a faker implementation for chrono's FixedOffset.

@llogiq
Copy link
Collaborator

llogiq commented Nov 12, 2023

Should we really allow minute-exact offsets? In the real world, there are mostly full hour offsets, with a scant few half hours strewn in.

@willtrnr
Copy link
Contributor Author

You're right, I conflated two use cases and forgot to make the data realistic. I guess half-hour increments would be reasonable and I'll clamp the range to -12 to +14.

@cksac
Copy link
Owner

cksac commented Nov 13, 2023

there are few offset at 15 / 45 mins from the list https://en.wikipedia.org/wiki/List_of_UTC_offsets#
I am not sure should we just random one from a fixed list?

@willtrnr
Copy link
Contributor Author

I though about the exception cases (Nepal comes to mind), but considering we are just generating realistic data and not every possible cases I figured sticking to the slightly more regular 30min slices is good enough.

@llogiq
Copy link
Collaborator

llogiq commented Nov 13, 2023

The problem with the list is that it isn't fixed – countries don't change their time zones weekly but every once in a while, so we'd either need to embed a time zone database or go around and update our list every time that happens.

@cksac cksac merged commit 4062db1 into cksac:master Nov 14, 2023
5 checks passed
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

Successfully merging this pull request may close these issues.

3 participants