Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace the geoip with a better-maintained package #91

Closed
grantholle opened this issue Feb 16, 2023 · 13 comments
Closed

Replace the geoip with a better-maintained package #91

grantholle opened this issue Feb 16, 2023 · 13 comments

Comments

@grantholle
Copy link
Contributor

When doing some Laravel 10 upgrades, the bottleneck for me right now is torann/laravel-geoip (after changing my composer's minimum-stability back to dev to get past #90). It seems minimally maintained.

Would it be worth switching to something like stevebauman/location. Steve is an amazing open source champion who maintains his packages very well.

I'd be happy to try and make the switch if you're up to it.

@grantholle
Copy link
Contributor Author

I began to start the PR, but decided that I was going to make too many breaking changes. I created my own package, grantholle/laravel-timezone, that only supports more modern Laravel and PHP versions. It also has different conventions and API.

If you're open to making breaking changes, then I can submit a PR. For now I'll close this issue.

@jamesmills
Copy link
Owner

Hi @grantholle

During my walk this morning I was thinking that I was going to tidy this package up and release a new breaking change version. This package is old now but has a lot of people using it. I think tidying it up and bringing some of the newer stuff to it makes sense.

I'll take a look at your clone.

James

@jamesmills
Copy link
Owner

First of all, I'm loving stevebauman/location. Looks really nice and well-maintained.

Secondly, you've now given me a huge conundrum. Your package is extremely well written and has cut out a lot of the stuff I was thinking of cutting out if I ever got time to release a new version.

I may abandon mine and link to yours.

@jamesautodude
Copy link

I began to start the PR, but decided that I was going to make too many breaking changes. I created my own package, grantholle/laravel-timezone, that only supports more modern Laravel and PHP versions. It also has different conventions and API.

If you're open to making breaking changes, then I can submit a PR. For now I'll close this issue.

Thank you sir. This was the last package I think I need to upgrade to laravel 10, and I use the timezone package very heavily so I can't just get rid of it. I'll use yours if that's okay with you, at least for now as I really wanted to upgrade ASAP. Thanks for your hard work!

@grantholle
Copy link
Contributor Author

I also rely heavily on this package, so I was motivated to find a solution. I'm fine with maintaining my package should you choose to abandon this one, or borrowing some for a new version. I appreciate your words of encouragement.

@usernotnull
Copy link
Contributor

If the plan is to move away from this package, kindly let us know ASAP since everyone is already doing the upgrading work of Laravel 10 :) I noticed there wouldn't be any other changes to the code than minor renaming when switching packages.

@jamesmills
Copy link
Owner

Nothing confirmed at this point. Too many people using this out in the wild to just abandon it.

@jamesautodude
Copy link

Nothing confirmed at this point. Too many people using this out in the wild to just abandon it.

So I guess if we want to upgrade to laravel 10 now, we should just use the package by @grantholle ?

@jamesmills
Copy link
Owner

If you’re already using this you should be ok upgrading to L10 using the latest tagged version of this package.

If you want a simplified and better written package then you can switch but it won’t support everything this one’s does for now.

Depends what you wanna do!

@jamesautodude
Copy link

If you’re already using this you should be ok upgrading to L10 using the latest tagged version of this package.

If you want a simplified and better written package then you can switch but it won’t support everything this one’s does for now.

Depends what you wanna do!

Interesting. I tried using your package and it still gives me errors :/ I'll try again tomorrow. Thanks!

@jamesmills
Copy link
Owner

I also rely heavily on this package, so I was motivated to find a solution. I'm fine with maintaining my package should you choose to abandon this one, or borrowing some for a new version. I appreciate your words of encouragement.

It would be nice to mix some of your stuff into a newer version of my current package if I’m perfectly honest with you.

People can continue to used previously tagged versions or they could upgrade/update and use a newer version of you’d be willing to PR some stuff.

I could add you as a maintainer?

@grantholle
Copy link
Contributor Author

Yeah sounds good.

@grantholle
Copy link
Contributor Author

Created #93

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants