New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use composer version of GeoIP library #12021

Merged
merged 2 commits into from Nov 19, 2017

Conversation

Projects
None yet
2 participants
@sgiehl
Member

sgiehl commented Sep 10, 2017

Our geoip library wasn't updated a long time. Meanwhile it is also available using composer. So why don't use that one :)

If we merge this, we should also update our build script to exclude all files besides those in src directory of the library

@mattab

This comment has been minimized.

Show comment
Hide comment
@mattab

mattab Sep 18, 2017

Member

Feedback:

  • In the commits there are big arrays of values which are not available in the latest version. Is this expected that these arrays of Region names, etc. are not anymore in the library?

  • maybe i'm missing something but not seeing the function geoip_country_code_by_addr_v6 and the other geoip_* functions in the library anymore, although we use them in core.

  • I don't think our tests cover all of GeoIP use cases, right? likely we need to manually test carefully the change

Member

mattab commented Sep 18, 2017

Feedback:

  • In the commits there are big arrays of values which are not available in the latest version. Is this expected that these arrays of Region names, etc. are not anymore in the library?

  • maybe i'm missing something but not seeing the function geoip_country_code_by_addr_v6 and the other geoip_* functions in the library anymore, although we use them in core.

  • I don't think our tests cover all of GeoIP use cases, right? likely we need to manually test carefully the change

@sgiehl

This comment has been minimized.

Show comment
Hide comment
@sgiehl

sgiehl Sep 20, 2017

Member
  • In the commits there are big arrays of values which are not available in the latest version. Is this expected that these arrays of Region names, etc. are not anymore in the library?
  • maybe i'm missing something but not seeing the function geoip_country_code_by_addr_v6 and the other geoip_* functions in the library anymore, although we use them in core.
  • I don't think our tests cover all of GeoIP use cases, right? likely we need to manually test carefully the change

The library is only removed from libs. running a composer install should install it in vendor. And there are those geoip methods and big arrays still included.

Member

sgiehl commented Sep 20, 2017

  • In the commits there are big arrays of values which are not available in the latest version. Is this expected that these arrays of Region names, etc. are not anymore in the library?
  • maybe i'm missing something but not seeing the function geoip_country_code_by_addr_v6 and the other geoip_* functions in the library anymore, although we use them in core.
  • I don't think our tests cover all of GeoIP use cases, right? likely we need to manually test carefully the change

The library is only removed from libs. running a composer install should install it in vendor. And there are those geoip methods and big arrays still included.

@mattab mattab modified the milestones: 3.3.0, 3.2.1 Oct 10, 2017

@sgiehl

This comment has been minimized.

Show comment
Hide comment
@sgiehl

sgiehl Oct 12, 2017

Member

I'll do a rebase of this branch to check if tests still passes. But imho it should be save to merge

Member

sgiehl commented Oct 12, 2017

I'll do a rebase of this branch to check if tests still passes. But imho it should be save to merge

@mattab mattab merged commit 23249e9 into 3.x-dev Nov 19, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@mattab mattab deleted the maxmindgeoip branch Nov 19, 2017

@sgiehl sgiehl restored the maxmindgeoip branch Nov 21, 2017

@sgiehl sgiehl deleted the maxmindgeoip branch Nov 21, 2017

sgiehl added a commit that referenced this pull request Nov 21, 2017

sgiehl added a commit that referenced this pull request Nov 22, 2017

@matomo-org matomo-org deleted a comment from MatomoForumBot Dec 4, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment