Alexkalanis/refactoring for php7 oop #8

alex-kalanis · 2022-10-15T02:16:16Z

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

- services into subdir, use factory for getting them - client also into subdir, allow to change to another one - add static analysis tools, allow add tests

- nearly all available code goes through tests

…hp7_oop Alexkalanis/refactoring for php7 oop

…s-php

dereuromark · 2024-02-20T01:25:58Z

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

yidas · 2024-02-21T05:16:41Z

There are two issues I'd like to discuss:

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

Thanks!

dereuromark · 2024-02-21T05:59:30Z

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.

# Conflicts: # README.md

alex-kalanis · 2024-02-21T06:34:57Z

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 · 2024-02-21T06:42:36Z

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.

yidas · 2024-02-21T06:55:05Z

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.

dereuromark · 2024-02-21T07:01:20Z

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

yidas · 2024-02-21T08:02:44Z

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.

…s-php into alexkalanis/refactoring_for_php7_oop

…s-php into alexkalanis/refactoring_for_php7_oop Necessary structure update and refactor

…s-php into alexkalanis/refactoring_for_php7_oop Updated services and tests

…s-php

alex-kalanis · 2024-02-29T15:18:36Z

Namespaces rewritten.

dereuromark · 2024-02-29T15:21:09Z

composer.json

@@ -10,9 +10,19 @@
        "source": "https://github.com/yidas/google-maps-services-php"
    },
    "require": {
-        "php": ">=5.4",
+        "ext-json": "*",
+        "php": ">=7.4",


dereuromark · 2024-02-29T15:21:59Z

src/Client.php

@@ -16,231 +15,68 @@
 * @method array elevation(string $locations, array $params=[])
 * @method array geocode(string $address, array $params=[])
 * @method array reverseGeocode(string $latlng, array $params=[])
- * @method array computeRoutes(array $origin, array $destination, array $body=[], array $fieldMask=[])
+ * @method array computeRoutes(array $origin, array $destination, array $body=[], array $headers=[], array $params=[])


That looks wrong

\yidas\googleMaps\Services\Routes::computeRoutes - src/Services/Routes.php:59

google-maps-services-php/src/Routes.php

Line 31 in a6300c7

public static function computeRoutes(Client $client, $origin, $destination, $body=[], $fieldMask=[])

My point is that there have been changes in upstream to provide fieldmask directly which seems reverted now in your PR.

dereuromark · 2024-02-29T15:23:15Z

.github/workflows/code_checks.yml

+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        php: [ '7.4', '8.0', '8.1' ]


Usually min max suffices
7.1, 8.3 then

My point is that only 7.1 and 8.3 needs testing

dereuromark · 2024-02-29T15:24:53Z

tests/ServiceTests/RoutesTest.php

+        $lib = new Services\Routes(new ApiAuth('test'));
+        $this->assertEquals('https://routes.googleapis.com/directions/v2:computeRoutes', $lib->getPath());
+        $this->assertEquals('POST', $lib->getMethod());
+        $this->assertEquals(null, $lib->getBody());


assertNull() usully

Small nitpick: also assertSame() with scalar values to make sure the type also mappes IMO

dereuromark · 2024-02-29T15:27:20Z

src/Services/Routes.php

+     * @throws ServiceException
+     * @return array<string, string|int|float>
+     */
+    public function computeRoutes($origin, $destination, $body=[], array $headers=[], array $params=[]): array


This looks different than upstream. I think headers are not used anymore afaik, instead fieldmask
See https://github.com/yidas/google-maps-services-php/pull/15/files

dereuromark · 2024-03-01T22:36:53Z

@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 · 2024-03-14T07:49:54Z

We should probably wrap this up :)

…s-php into alexkalanis/refactoring_for_php7_oop

dereuromark · 2024-03-24T12:20:54Z

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

alex-kalanis and others added 18 commits October 15, 2022 00:47

Rewrite for version 2 - use more object-oriented approach

c08b2bc

- services into subdir, use factory for getting them - client also into subdir, allow to change to another one - add static analysis tools, allow add tests

Updated by phpstan response

010a5dd

Create some tests

fa39653

- nearly all available code goes through tests

Update readme for added service

8af4c75

Tests of response content - need explicit array

c55e29e

phpstan about callbacks

2980765

phpstan about callbacks 2

2a6d60e

Merge pull request #1 from alex-kalanis/alexkalanis/refactoring_for_p…

c7123e6

…hp7_oop Alexkalanis/refactoring for php7 oop

Git attributes for composer package cleanup

14bb989

Use reflection to make instances of classes in Factory

4bbcbbb

Update param comments

c5fb703

Merge branch 'alexkalanis/refactoring_for_php7_oop'

9dc2be6

Merge branch 'master' of https://github.com/yidas/google-maps-service…

ff6143f

…s-php

Phpunit limitation

0aeeda4

More clients available, with tests

b9c2044

Changes in Composer

d087282

Limit things passing through

97ba32b

Parsed link must be recognizable

b306593

dereuromark mentioned this pull request Feb 20, 2024

Release 0.1 php-collective/google-maps-services#2

Open

alex-kalanis added 3 commits February 20, 2024 14:00

Extending API - added finder for place and services, info about points

0e72b3e

Extend readme

9676b6f

Readme links

622a8d9

Merge remote-tracking branch 'origin'

ba56087

# Conflicts: # README.md

alex-kalanis added 6 commits February 21, 2024 15:41

Merge branch 'master' of https://github.com/yidas/google-maps-service…

8db12be

…s-php into alexkalanis/refactoring_for_php7_oop

Merge branch 'master' of https://github.com/yidas/google-maps-service…

5069551

…s-php into alexkalanis/refactoring_for_php7_oop Necessary structure update and refactor

ApiAuth not need to have that link constant

bcb19cc

Psr client with its support

91e755f

Merge branch 'master' of https://github.com/yidas/google-maps-service…

3560be9

…s-php into alexkalanis/refactoring_for_php7_oop Updated services and tests

Phpstan class namespace

be15e93

dereuromark mentioned this pull request Feb 28, 2024

Support PSR HTTP client #9

Open

Merge branch 'master' of https://github.com/yidas/google-maps-service…

8f52ad0

…s-php

dereuromark reviewed Feb 29, 2024

View reviewed changes

alex-kalanis added 3 commits March 24, 2024 12:37

Updated in accordance with notes from dereuromark

f7ba6a3

Merge branch 'master' of https://github.com/yidas/google-maps-service…

63db423

…s-php into alexkalanis/refactoring_for_php7_oop

Added prepared check

891006b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Alexkalanis/refactoring for php7 oop #8

Alexkalanis/refactoring for php7 oop #8

alex-kalanis commented Oct 15, 2022

dereuromark commented Feb 20, 2024 •

edited

yidas commented Feb 21, 2024

dereuromark commented Feb 21, 2024

alex-kalanis commented Feb 21, 2024

alex-kalanis commented Feb 21, 2024

yidas commented Feb 21, 2024

dereuromark commented Feb 21, 2024

yidas commented Feb 21, 2024

alex-kalanis commented Feb 29, 2024

dereuromark Feb 29, 2024

dereuromark Feb 29, 2024

alex-kalanis Mar 24, 2024

dereuromark Mar 24, 2024

dereuromark Feb 29, 2024

alex-kalanis Mar 24, 2024

dereuromark Mar 24, 2024

dereuromark Feb 29, 2024

dereuromark Feb 29, 2024

dereuromark commented Mar 1, 2024

dereuromark commented Mar 14, 2024

dereuromark commented Mar 24, 2024

Alexkalanis/refactoring for php7 oop #8

Are you sure you want to change the base?

Alexkalanis/refactoring for php7 oop #8

Conversation

alex-kalanis commented Oct 15, 2022

dereuromark commented Feb 20, 2024 • edited

yidas commented Feb 21, 2024

dereuromark commented Feb 21, 2024

alex-kalanis commented Feb 21, 2024

alex-kalanis commented Feb 21, 2024

yidas commented Feb 21, 2024

dereuromark commented Feb 21, 2024

yidas commented Feb 21, 2024

alex-kalanis commented Feb 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dereuromark commented Mar 1, 2024

dereuromark commented Mar 14, 2024

dereuromark commented Mar 24, 2024

dereuromark commented Feb 20, 2024 •

edited