google-maps-services-php icon indicating copy to clipboard operation
google-maps-services-php copied to clipboard

Published 20 hours ago • verified publisher icon yidas

Alexkalanis/refactoring for php7 oop

Open alex-kalanis opened this issue 2 years ago • 12 comments

Update nearly the complete code, possible to use as v2, minimal php7

  • added service to access Nearby endpoint (my inital reason)
  • enabled extending of some parts (extended service factory, other query libraries)
  • added tests (phpunit, phpstan, codestyle)
  • added comments and property descriptions

alex-kalanis avatar Oct 15 '22 02:10 alex-kalanis

Wow, this looks super useful I love that you also added tests

Would you be interested in re-added this to the maintained fork @ https://github.com/php-collective/google-maps-services ?

  • CI
  • tests
  • new functionality

Note:

  • CS is already added, so this can be skipped.
  • We can rename the namespace from yidas\googleMaps to PhpCollective\GoogleMaps

dereuromark avatar Feb 20 '24 01:02 dereuromark

There are two issues I'd like to discuss:

  1. Modifying the mininal PHP version to 7.4 is embarrassing, I suggest to keep supporting 7.0 as the current one.
  2. Modifying the namespace from lower to upper camel case is the same as above.

Thanks!

yidas avatar Feb 21 '24 05:02 yidas

7.4 is definitly the best min version possible at this point (2024). Even 8.0 is already EOL - https://www.php.net/supported-versions.php Whats the reason to support versions from like 4 years ago?

I agree that the namespace change looks like a BC break and as such should not be done without majoring, so probably not in this PR.

dereuromark avatar Feb 21 '24 05:02 dereuromark

7.1 is minimum due type control over null. "?string" etc... Even PSR already have problems with php7 (here http message).
API in src\Client stays the same.

alex-kalanis avatar Feb 21 '24 06:02 alex-kalanis

Also I do not expect that this pull request will rot here for 2 years. I set major version in mine repo as 2 - breaking change is then expected.

alex-kalanis avatar Feb 21 '24 06:02 alex-kalanis

The idea is to take care of users of the ancient system who want to use new services as possible.

How about to keep the mininal PHP version at 7.1 so far with the same namespace.

yidas avatar Feb 21 '24 06:02 yidas

There should be no one anymore: https://stitcher.io/blog/php-version-stats-january-2024 Ancient is basically 7.4 (LTS for 7.x)

dereuromark avatar Feb 21 '24 07:02 dereuromark

I also referred to that article which revealed the number of 7.1 releases until January 2024.

from my perspective, the priority we can work on right now is to check which code changes are using the 7.4 version restrictions.

yidas avatar Feb 21 '24 08:02 yidas

Namespaces rewritten.

alex-kalanis avatar Feb 29 '24 15:02 alex-kalanis

@yidas I recommend changing the github setting so only "new github contributors" require a manual workflow run init. This way, the tests will run right away for PRs.

dereuromark avatar Mar 01 '24 22:03 dereuromark

We should probably wrap this up :)

dereuromark avatar Mar 14 '24 07:03 dereuromark

@yidas you still would want to change the github action run setting as per my suggestion

dereuromark avatar Mar 24 '24 12:03 dereuromark