search

edulify / play-geolocation-module.edulify.com

This is a geolocation plugin for Playframework
https://edulify.github.io/play-geolocation-module.edulify.com/
Apache License 2.0
22 stars 11 forks source link

Added timezone for Freegeoip #2

Closed shanness closed 9 years ago

shanness commented 9 years ago

Hey Megazord and team, I've added timeZone to the freegeoip provider and the geocode model. Seems like a simple and worthy addition, and I need it for a project.

Thanks for a great project!

Cheers,

Craig

shanness commented 9 years ago

Hmm, I hadn't even thought of using the TimeZone field. There might be errors if it's blank or invalid. Actually, looking at the TZ code in java 8. public static synchronized TimeZone getTimeZone(String ID) { return getTimeZone(ID, true); } The true means return GMT if the ID isn't found, and there is no way to pass in false (the getTimeZone(ID,boolean) method is private). It seems like it could cause weird problems using the TimeZone class, and the String is all I need for my purposes. What are your thoughts? SimpleTimeZone requires an offset (which isn't in the data)

megazord commented 9 years ago

@shanness returning GMT if no timezone was specified looks like a good default to me. But I'm not sure about how it resonates to other users.

So, I think we can go with a String right now (since users can easily convert it to a java.util.TimeZone if necessary).

megazord commented 9 years ago

I will do a release at the end of the day. GMT -3:00 here. :-)

shanness commented 9 years ago

@megazord Thanks! Yeah, I'm worried that GMT isn't a great default, as it is a timezone, and not the right one, hence would stuff up calculations.. I'd prefer to just not have one and know it. And I agree it's easy to convert. Cheers!

megazord commented 9 years ago

@shanness version 1.4.1 released!

shanness commented 9 years ago

Great, thanks @megazord!