Skip to content

Allow willdurand/geocoder 5.0 for PHP 8.4+#4

Merged
filisko merged 2 commits into
middlewares:masterfrom
steffans:patch-1
Aug 4, 2025
Merged

Allow willdurand/geocoder 5.0 for PHP 8.4+#4
filisko merged 2 commits into
middlewares:masterfrom
steffans:patch-1

Conversation

@steffans

Copy link
Copy Markdown
Contributor

No description provided.

@oscarotero oscarotero requested a review from filisko July 28, 2025 13:33
@filisko

filisko commented Jul 28, 2025

Copy link
Copy Markdown
Member

@oscarotero thanks for tagging me. I think I removed my notifications by mistake.

@steffans in reality this will be applied for PHP >=8.2, not for 8.4 only
Let me give it a thought

@filisko

filisko commented Jul 28, 2025

Copy link
Copy Markdown
Member

@steffans @oscarotero just enabled the CI and seems like the JSON is wrong lol . A comma is missing at the end of the line

@steffans

Copy link
Copy Markdown
Contributor Author

It allows to install geocoder 5.0 which brings PHP 8.4 compatibility, otherwise this middleware remains incompatible with the latest PHP version.

@filisko

filisko commented Aug 4, 2025

Copy link
Copy Markdown
Member

@steffans that's not true but it's needed

The constraint for v4 is php: ^7.4 || ^8.0 (so technically it supports all php8 versions) or do you mean that it actually breaks?

@filisko

filisko commented Aug 4, 2025

Copy link
Copy Markdown
Member

@oscarotero @steffans so my thoughts are these:

For those users who installed the geocoder lib as a side effect of installing this middleware, their code will break if they use any PHP 8 version because installing this library made them install the v4 of the geocoder library, and unless they manually installed the geocoder library (gain control of its version), when they upgrade to the next release, similarly, the geocoder lib will be upgraded to v5 thus making this a breaking change.

And given that probably most users installed the v4 as a side effect, if they have customised code for the geocoder lib, it will break.

So I think the best is to create a new major version of this middleware (v4). @oscarotero share thoughts.

@oscarotero

Copy link
Copy Markdown
Member

@filisko

Looking at the changelog, seems that there are no breaking changes, only a deprecated method and drop support for old PHP versions. So maybe a major version is not necessary.

@filisko

filisko commented Aug 4, 2025

Copy link
Copy Markdown
Member

@oscarotero I guess deprecating that method is a Breaking change already?

@steffans

steffans commented Aug 4, 2025

Copy link
Copy Markdown
Contributor Author

@filisko The geocoder 5.0 release fixes PHP 8.4: Implicitly nullable parameter declarations deprecated messages

@oscarotero

Copy link
Copy Markdown
Member

@filisko if the method is not removed, I wouldn't consider it a breaking change. It's like when PHP deprecates some functions between minor versions, and remove them after a major version.
In fact, this change was initially intended for a minor version of V4 (geocoder-php/Geocoder#1184).

In my opinion, a minor version is okay, but I leave the final decision to you, that you have more context. If you think a major version is better, I'm okay with that too.

@filisko

filisko commented Aug 4, 2025

Copy link
Copy Markdown
Member

@oscarotero all right, makes sense if it's only the notice! Thanks! Will release it today ASAP

@filisko filisko merged commit a4b971e into middlewares:master Aug 4, 2025
9 checks passed
@filisko

filisko commented Aug 4, 2025

Copy link
Copy Markdown
Member

@steffans thanks!

@filisko

filisko commented Aug 4, 2025

Copy link
Copy Markdown
Member

@oscarotero @steffans ah, one thing that I thought about why a major is needed is to rename the package dependency to geocoder-php/geocoder.

@oscarotero

Copy link
Copy Markdown
Member

I'm a bit confused about existing two geocoder packages?
As far as I can see, willdurand/geocoder is the base package with all common classes and interfaces. As long as this middleware is provider-agnostic (you can use any of the available providers with it), maybe it's the right packages? I'm not sure what geocoder-php/geocoder is intended for. The package description says "A development package for all providers".

@filisko

filisko commented Aug 4, 2025

Copy link
Copy Markdown
Member

@oscarotero I mean with this one https://github.com/geocoder-php/Geocoder

If you go to the current package https://packagist.org/packages/willdurand/geocoder It says "READONLY" 🤔

@oscarotero

oscarotero commented Aug 4, 2025

Copy link
Copy Markdown
Member

@filisko No idea.
We can release a minor version with willdurand/geocoder for now, and decide later whether we have to release a new major version with a different dependency.

@filisko

filisko commented Aug 5, 2025

Copy link
Copy Markdown
Member

@oscarotero @steffans just released https://github.com/middlewares/geolocation/blob/v3.2.0/CHANGELOG.md :)

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.

3 participants