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

Preserve the timezone of the source when returning the time object #40

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

manu3569
Copy link

Thanks for this handy gem.

For my use case, I needed to be able to support a source time with a time zone that is different from the local one. This change preserves the time zone on the returned time object.

This breaks the use case where an alternate time_source class is passed in that does not support the new method with the same signature as the one for Time.new.

@mperham
Copy link
Contributor

mperham commented Nov 18, 2021

AFAICT this PR also fixes a major bug: the cron parser skips the "fallback" hour of DST because the next method jumps from "00:59:00 PDT" to "01:00:00 PST" as it does not track the timezone. With this PR:

2021-11-07 00:55:00 -0700
2021-11-07 00:56:00 -0700
2021-11-07 00:57:00 -0700
2021-11-07 00:58:00 -0700
2021-11-07 00:59:00 -0700
2021-11-07 01:00:00 -0700
2021-11-07 01:01:00 -0700
2021-11-07 01:02:00 -0700
2021-11-07 01:03:00 -0700
...snip...
2021-11-07 01:55:00 -0700
2021-11-07 01:56:00 -0700
2021-11-07 01:57:00 -0700
2021-11-07 01:58:00 -0700
2021-11-07 01:59:00 -0700
2021-11-07 02:00:00 -0700
2021-11-07 01:01:00 -0800
2021-11-07 01:02:00 -0800
2021-11-07 01:03:00 -0800
2021-11-07 01:04:00 -0800

@mperham
Copy link
Contributor

mperham commented Nov 18, 2021

Without the PR:

2021-11-07 00:55:00 -0700
2021-11-07 00:56:00 -0700
2021-11-07 00:57:00 -0700
2021-11-07 00:58:00 -0700
2021-11-07 00:59:00 -0700
2021-11-07 01:00:00 -0800
2021-11-07 01:01:00 -0800
2021-11-07 01:02:00 -0800
2021-11-07 01:03:00 -0800
...snip...
2021-11-07 01:57:00 -0800
2021-11-07 01:58:00 -0800
2021-11-07 01:59:00 -0800
2021-11-07 02:00:00 -0800
2021-11-07 01:01:00 -0800
2021-11-07 01:02:00 -0800
2021-11-07 01:03:00 -0800
2021-11-07 01:04:00 -0800

@manu3569
Copy link
Author

Awesome. Glad to hear that the timezone addition fixes DST issues.

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.

2 participants