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.1k stars 373 forks source link

Undefined method reference... #152

Closed PhoenixPeca closed 5 years ago

PhoenixPeca commented 5 years ago

Hi,

I would just like to point out a reference to a method that does not (or no longer) exists..

https://github.com/Torann/laravel-geoip/blob/d00e9b5b9c8e9c79c47fc4d78d4b47db1cd532c3/src/GeoIP.php#L164

image

Possible fix: https://github.com/Seldaek/monolog/commit/d686f25f6c1ce9706e23cf1c6f97c76e8bf83d09

imami commented 5 years ago

This is because addError method no longer exists in Monolog v. 2.0. Maybe some major upgrade in laravel-geoip is needed!

@Torann

PhoenixPeca commented 5 years ago

Correct me if I'm wrong, but as I can see, the only usage of monolog in this repo is found in these lines: https://github.com/Torann/laravel-geoip/blob/dc3c4fc17779b3521736dd4e04d1fa2dd8f55db9/src/GeoIP.php#L162-L164

so, i dont think a major upgrade is needed.. I have made a PR already for this: #153

imami commented 5 years ago

@PhoenixPeca You're correct

Torann commented 5 years ago

Looks like addError() just needs to be changed to error().

I'm also 100% fine with dropping the logger from here. I never use it.

Torann commented 5 years ago

LOL I just saw that you made the changes and put up a PR @PhoenixPeca. I'll merge it now 👍

Torann commented 5 years ago

Hold up. This change is breaking. I'll make a comment on the PR for a fix to prevent this

Torann commented 5 years ago

Ok. I just looked, v1.x of Monolog also has the method error, so this will not break anything. Merging now :-)