morris / vinyl-ftp

Blazing fast vinyl adapter for FTP
Other
388 stars 31 forks source link

Convert UTC time to local time; Close #90 #103

Open JacobDB opened 6 years ago

JacobDB commented 6 years ago

Implemented @nandaleite's solution to issue #90. Tested in Node v6 and in v8, it's working fine in both instances.

JacobDB commented 6 years ago

Tested in Node v9 today, no issues there either.

morris commented 6 years ago

Could you add tests to verify this works? I'm unsure if the date arithmetic will work. getHours() - 1 especially has a few edge cases. Thanks!

JacobDB commented 6 years ago

Unfortunately, I'm really not much of an expert when it comes to Node JS development; if you can point me in the right direction I'd be happy to add some tests, I'm just not sure where to even start with something like that.

arktds commented 5 years ago

As far as I see, this works seems good. If date.getTimezoneOffset() returns 60 in UTC+1, then var timezone = date.getTimezoneOffset() / 60 // = 1. Even in this case, obj.date = date.setHours(date.getHours() - timezone); should works properly.

morris commented 5 years ago

What happens for date.getHours() === 0? Then obj.date = date.setHours(-1)? If you can provide a test I'd be happy to merge :)

urosg80 commented 4 years ago

According to documentation this ensures that the date is also changed - i.e. last hour of previous day; also in posted example it is shown how to "move" time 48 hours back (in the example).

Actually after testing this I found out that this fix doesn't work correctly.

The correct way to set UTC time (as local stat times are also in UTC) is not to mess with timezoneoffset and sethour etc. at all. Just include "+00:00" at the end of the date/time string when composing the new Date object.

127 fixes this