Change from single provider to chain provider #8
+31
−8
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for the PR I appreciate it ! I see your point and I can see that can be usefull even it's a BC break :) |
|
Actually there would be no BC break, since ChainProvider acts as any provider, Geocoder just calls them in the order given and the first result returned is the one that Geocoder uses. As long as FreeGeoIp is listed as the first one, existing functionality would remain the same. I'll reply on #5 about the config thing. |
|
You're right no BC :) |
|
I'm quite interested in this. can it be added to dev-master? |
|
@dwenaus yes :) |
toin0u
added a commit
that referenced
this pull request
May 12, 2014
Change from single provider to chain provider
|
Are you sure you're using dev-master ? I have no BC break. And units tests looks good to me in order to test BC. |
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The ChainProvider is by far the most useful. This PR changes the package default to use it, with FreeGeoIpProvider as the default single member of the chain. Client code can then add additional providers just by adding to the providers config array.
Tests are updated accordingly. Because ChainProvider::$providers is protected, I had to add a test method to grab it with ReflectionClass. This feels a little wrong, but I didn't see a better place to put it in your existing test setup. Let me know if this should be revised.