feature: remove all non-https support #567
Conversation
|
@Nyholm test pass locally with php 5.6, do I have to do something about the coordinates ? |
Nyholm
left a comment
Nyholm
left a comment
There was a problem hiding this comment.
Great PR. Just remove some comments and fix that test.
| /** | ||
| * @param HttpClient $client An HTTP adapter | ||
| * @param string $sourceCountry Country biasing (optional) | ||
| * @param bool $useSsl Whether to use an SSL connection (optional) |
src/Geocoder/Provider/MaxMind.php
Outdated
| * @param HttpClient $client An HTTP adapter. | ||
| * @param string $apiKey An API key. | ||
| * @param string $service The specific Maxmind service to use (optional). | ||
| * @param bool $useSsl Whether to use an SSL connection (optional). |
| { | ||
| $provider = new GoogleMaps($this->getAdapter()); | ||
| $results = $provider->geocode('Paris'); | ||
| $results = $provider->geocode('i'); |
There was a problem hiding this comment.
This line should not be changed. Keep "Paris" here and we do not have to modify the rest of the test.
There was a problem hiding this comment.
If I let Paris, the test does not pass anymore because of https://maps.googleapis.com/maps/api/geocode/json?address=Paris%22 returning just one element
There was a problem hiding this comment.
That is very strange... But the issue is at the geocoder service. Let's do as you do an remove 'Paris'.
There was a problem hiding this comment.
Ah, you get new caches.. Please rename the file "tests/.cached_responses/0e41628de6d90aa50c426e12391617abc3ccf5ff" to "tests/.cached_responses/65760b8fc8d216f997a2e5b2ce499881426f7353".
Now it will work with "Paris"
No, you should not change any of the coordinates or addresses. I think you accidentally modified the geocode string. I wrote an inline comment about that. |
Simperfit
force-pushed
the
feature/remove-non-ssl
branch
from
ab67066 to
3fd7fe2
Compare
|
I think the ApiResult of Arcgis are inconsistent, since it does not break on the same things everytime |
|
True. Run your tests locally. Make sure that you have the following in your phpunit.xml If the test passes, make sure to checkin the new cached response that will be created in |
|
From #567 (comment)
|
|
@Nyholm done ;) |
Nyholm
left a comment
Nyholm
left a comment
There was a problem hiding this comment.
Looks great. Had a small question. Doing a final review tomorrow. (Guest just arrived)
| $this->assertInstanceOf('\Geocoder\Model\Address', $result); | ||
| $this->assertEquals(33.6609389, $result->getCoordinates()->getLatitude(), '', 0.001); | ||
| $this->assertEquals(-95.555513, $result->getCoordinates()->getLongitude(), '', 0.001); | ||
| $this->assertEquals(33.660938899999998, $result->getCoordinates()->getLatitude(), '', 0.001); |
There was a problem hiding this comment.
Why do you need to change these coordinates?
There was a problem hiding this comment.
You are right, I don't need too ;)
|
Thank you @Simperfit. Great work! |
2 participants
fix #562