search

Torann / laravel-geoip

Determine the geographical location of website visitors based on their IP addresses.
http://lyften.com/projects/laravel-geoip
BSD 2-Clause "Simplified" License
2.07k stars 372 forks source link

Fix MaxMind database fetching #164

Closed dwightwatson closed 4 years ago

dwightwatson commented 4 years ago

This is a follow-up to #163 that is intended to fix #162.

It expands on the work by @tylermann - with a couple of differences.

Instead of adding the database filename to the config it searches for the first file with the extension .mmdb and assumes this is the correct one. Seems like a little simpler to me though less configurable - open to feedback here.

It would be great if we could leverage Laravel's filesystem adapter as it would clean up much of the directory creation/removal code, but that's probably part of a larger discussion.

dwightwatson commented 4 years ago

This is likely to still fail because there is no licence key on the testing server - so it's a matter of getting a licence key for testing, stubbing out the download or removing the test completely.

theblindfrog commented 4 years ago

@Torann are you able to review this PR?

Torann commented 4 years ago

Going over it today

Torann commented 4 years ago

This looks great!!! I'm thinking since people have to update their config file, which is out of our hands, I'm going to mark this a version 1.1 which will then require people to go over the upgrade doc. @dwightwatson if you wouldn't mind could you update upgrade.md on Lyften.com with the best way for people to go about upgrading from v1.0 to v1.1

How does this sound?

dwightwatson commented 4 years ago

Makes sense, no worries - just made a quick PR that goes over the change: https://github.com/Torann/lyften.com/pull/17

Torann commented 4 years ago

Ok, making the release version now and the docs are being deployed now. Thanks!