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

Enable timestamps to use alternative hours #46

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

autotelik
Copy link

No description provided.

@jheth
Copy link
Contributor

jheth commented Sep 22, 2016

Hey @autotelik. Thanks for the contribution! I had been thinking about a configuration object like this. I'm trying to avoid dependencies on rails for this gem and I see you reference ActiveSupport. Is there a way to avoid that?

The problem you're solving feels like clock-drift or time-zone. I'd rather not have two properties for house and minutes. I'd prefer to either have a clock-drift property which would be minutes or a timezone attribute that would compare based on timestamp. All timestamps from the gem right now are UTC, which is lowest common denominator.

Can you explain your scenario more?

@autotelik
Copy link
Author

hi ... really, sorry, no idea how this ended up here .. this was supposed to just be a local fix in our fork

@autotelik
Copy link
Author

for info we had some funny issues on Ubuntu where CRM would reject messages due to timestamp issue but could not resolve via setting the various times in Linux.

There did not seem a way to trigger usage of your original methods like get_current_time_plus_hour so was an attempt to create a configurable way to over ride the defaul timestamps

@jheth
Copy link
Contributor

jheth commented Nov 5, 2016

@autotelik I like the idea of making timing checks being expanded but these properties don't feel quite right. I like the configuration object but we shouldn't need rails for it.

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