Skip to content

Support Laravel 12 and Future Version#67

Merged
silvanocerza merged 2 commits into
ipinfo:masterfrom
matriphe:laravel-future-version
Aug 19, 2025
Merged

Support Laravel 12 and Future Version#67
silvanocerza merged 2 commits into
ipinfo:masterfrom
matriphe:laravel-future-version

Conversation

@matriphe
Copy link
Copy Markdown
Contributor

@matriphe matriphe commented Apr 6, 2025

This PR removes the limitation to support future Laravel version.

This will also address issue #66.

@matriphe matriphe marked this pull request as ready for review April 6, 2025 08:43
@farzanahmad
Copy link
Copy Markdown

Can someone please merge this PR?

@max-ipinfo
Copy link
Copy Markdown
Contributor

Thank you @matriphe @farzanahmad @3m1n3nc3 for the PR and your comments.

I'm not a Laravel user so I don't know whether it's best practice to openly support future versions. Can you point me at best practices / examples for Laravel libraries and versioning?

I don't have the full historical context on our library either, but I do know that we typically go though a manual test of every Laravel version before we state that we support it in our package. That may the reason why we added this version range constraint.

I'll start a conversation internally and will get back to you about the next steps.

@matriphe
Copy link
Copy Markdown
Contributor Author

In Laravel, the illuminate/* versions usually folllow Laravel's version because they are subtree. for example, in Laravel 12, the laravel/framework dependency will have version ^12.0 and the illuminate/* will point to the same version.

it also usually has different version for the require-dev part, especially on the phpunit/phpunit, but I think this is a minor part and will not prevent to upgrade.

because this library only uses illuminate/support to build the Facade, I think it's pretty safe to always follow the Laravel version. the only thing that might break is the PHP version, but I think you need to upgrade the PHP version to satisfy the Laravel requirement anyway.

honestly, we don't really need this library, because I can still use the ipinfo/ipinfo directly, but having access to the Facade from illuminate/support in Laravel is a nice feature. that's why I use this library on my Laravel projects.

Testing

I see there is a test part in here, but I don't think it does something. it would be beneficial if we have an automated test using GitHub Actions to make sure it's compatible with the Laravel version. for testing, we can use Orchestral for this, since it doesn't need to load all the Laravel application.

I can also help with setting up the GHA and the test, but the test scenario might a little bit tricky. ideally, we can have a kind of "integration test", where it makes a call to a "test server" using a special access token, that will always return consistent data, then the test will verify the result to make sure it gets all the data from IPInfo correctly by comparing it. otherwise, we only rely on a mocked data in the unit test.

but this testing part is out of this discussion.

@matriphe
Copy link
Copy Markdown
Contributor Author

we can also make the requirements to illuminate/support: * to not care about the Laravel version, but I think it's not a good thing. if it breaks, we will not know on which Laravel version that breaks. this will also make us follow the Laravel development, and maintain the library and not forget about this.

@max-ipinfo
Copy link
Copy Markdown
Contributor

We will be attempting to merge this soon, releasing it with #68.

@silvanocerza can you handle merging this PR?

@silvanocerza silvanocerza merged commit 6978c2a into ipinfo:master Aug 19, 2025
@silvanocerza
Copy link
Copy Markdown
Contributor

Forgot to approve it before merging, still does the job. 👍

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

Successfully merging this pull request may close these issues.

4 participants