Conversation
|
Thank you for your PR. This all comes new to me, I haven't though about a feature like a time offset yet. What would be the benefit of introducing a time offset option compared with the timezone? I like the clean code you submitted, and the tests, and everything else, I'm just not 100% sure this feature belongs here and not in a different time management package. |
|
Honestly, for my use case timezone selection would've been enough (development VM with incorrect timezone and changing PC's timezone is highly undesired at this time). But when looking at how it can be implemented I figured offset in minutes is way more flexible and requires less maintenance in the long run. We don't need to worry about timezones of some countries changing in the future or about daylight saving time or about keeping timezone libraries up to date. Ultimately, the decision is up to you.
Thank you. Only I forgot to update documentation. Give me a heads up if you'll want to merge this PR, so I can update the documentation before the merge. |
|
Sure thing, thanks a lot for that. I will need some time to dive again into the project, as it's been a while since the last time I worked on it, but I remember I had something in place for timezones using |
Implementing it with
moment-timezonewould've been prettier, but this way is simpler, more flexible and adds no extra dependencies.