Allow offsets and transitions to be retrieved from Timezone #1

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
3 participants
@davispuh

There wasn't any way to get all offsets nor transitions so I added these methods.

Now it's possible to do so
TZInfo::Timezone.get("Europe/Paris").all_offsets which will return array of TimezoneOffsetInfo's

+
+ # Returns array of all TimezoneTransitionInfo for this Timezone.
+ def all_transitions
+ info.transitions.freeze

This comment has been minimized.

@davispuh

davispuh Aug 15, 2013

we freeze it so user doesn't accidentally modify it as it might have bad side effects. Only it means that after call to all_transitions it's not possible to add new transitions. But it shouldn't be a problem because usually you would add all transitions at beginning.

@davispuh

davispuh Aug 15, 2013

we freeze it so user doesn't accidentally modify it as it might have bad side effects. Only it means that after call to all_transitions it's not possible to add new transitions. But it shouldn't be a problem because usually you would add all transitions at beginning.

@davispuh

This comment has been minimized.

Show comment
Hide comment
@davispuh

davispuh Aug 20, 2013

I just rebased, there wasn't any conflicts.

I just rebased, there wasn't any conflicts.

@philr

This comment has been minimized.

Show comment
Hide comment
@philr

philr Aug 26, 2013

Member

Thank you for submitting this pull request.

I've actually been working on something similar and have just pushed my changes to master. The key difference between your and my changes is that I'm only supporting getting the transitions and offsets for a range of time.

The reason for requiring the range is that I'm planning to add support for including rules that define indefinitely repeating future transitions. This would mean that there would be an infinite number of transitions for many of the timezones (making returning all of them impossible!).

The main change (adding transitions_up_to and offsets_up_to to Timezone) is in 3f1364e. I've also made a few additional changes to clean up (and rename) the TimezoneTransitionInfo and TimezoneOffsetInfo classes now that they are available as part of the public interface.

Member

philr commented Aug 26, 2013

Thank you for submitting this pull request.

I've actually been working on something similar and have just pushed my changes to master. The key difference between your and my changes is that I'm only supporting getting the transitions and offsets for a range of time.

The reason for requiring the range is that I'm planning to add support for including rules that define indefinitely repeating future transitions. This would mean that there would be an infinite number of transitions for many of the timezones (making returning all of them impossible!).

The main change (adding transitions_up_to and offsets_up_to to Timezone) is in 3f1364e. I've also made a few additional changes to clean up (and rename) the TimezoneTransitionInfo and TimezoneOffsetInfo classes now that they are available as part of the public interface.

@philr philr closed this Aug 26, 2013

@davispuh

This comment has been minimized.

Show comment
Hide comment
@davispuh

davispuh Aug 26, 2013

Nice :) 👍

Nice :) 👍

@benhass

This comment has been minimized.

Show comment
Hide comment
@benhass

benhass Oct 14, 2014

@philr, any chance you could backport the methods added in 3f1364e to ~> V0.3.38 to support rails 3.2?

benhass commented Oct 14, 2014

@philr, any chance you could backport the methods added in 3f1364e to ~> V0.3.38 to support rails 3.2?

@philr

This comment has been minimized.

Show comment
Hide comment
@philr

philr Oct 16, 2014

Member

@benhass I'm only doing data updates and bug fixes on the 0.3.x branch now, so I'm not going to backport this change.

If you want to, it looks like it would be reasonably straight-forward to monkey patch this change onto v0.3.38. I think all the dependencies will be there, so it should just be a case of adding the new methods. Note that the transitions_up_to method added to TransitionDataTimezoneInfo in 3f1364e would actually need to be added to the DataTimezoneInfo class in 0.3.

Member

philr commented Oct 16, 2014

@benhass I'm only doing data updates and bug fixes on the 0.3.x branch now, so I'm not going to backport this change.

If you want to, it looks like it would be reasonably straight-forward to monkey patch this change onto v0.3.38. I think all the dependencies will be there, so it should just be a case of adding the new methods. Note that the transitions_up_to method added to TransitionDataTimezoneInfo in 3f1364e would actually need to be added to the DataTimezoneInfo class in 0.3.

@benhass

This comment has been minimized.

Show comment
Hide comment
@benhass

benhass Oct 16, 2014

Fair enough @philr, it's probably just time we upgrade, though I'll consider monkey-patching this if need be. Thanks for getting back to me.

benhass commented Oct 16, 2014

Fair enough @philr, it's probably just time we upgrade, though I'll consider monkey-patching this if need be. Thanks for getting back to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment