-
Notifications
You must be signed in to change notification settings - Fork 81
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
Enable the use of MaxMind GeoIP2-Domain databases #162
Conversation
Hi @Sitwon thanks for you contribution, before anybody could start to review, please sign the committer license agreement, CLA. Thanks |
Hi, I signed the CLA and downloaded the PDF copy. Is there another step I
need to take?
…On Thu, Feb 6, 2020, 03:53 Andrea Selva ***@***.***> wrote:
Hi @Sitwon <https://github.com/Sitwon> thanks for you contribution,
before anybody could start to review, please sign the committer license
agreement, CLA. Thanks
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#162?email_source=notifications&email_token=AAAXV6AL4BINPXI53QJCCLDRBPFZBA5CNFSM4KQN3XWKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEK6NAPQ#issuecomment-582799422>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAAXV6FUYAYSNF3L2MZATDTRBPFZBANCNFSM4KQN3XWA>
.
|
I see, I just had to amend the commit author to use the same email address I gave for the CLA. |
@andsel I've signed the CLA. The build failure in Travis does not seem to be related to my change. It looks like Travis timed out trying to download https://services.gradle.org/distributions/gradle-5.6.4-bin.zip. |
#174 is asking for this to be merged. I'm still not clear on what's holding it up. The Travis CI failure is unrelated to this change. |
@Sitwon Thank you for taking the time to contribute and to following up with this PR, and sorry for the delayed response. @kaisecheng will review your PR and approve/request changes as appropriate! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Sitwon Thank you for enhancing GeoIP filter. Overall looks good to me. Sorry for the long waiting time. CI red is due to another issue. GeoIP2-Domain is a paid database, thus no test database for test case.
Could you update the change log and the version number ? Once it updates, I can merge it
In addition to the location databases, MaxMind provides a database of domain data as well. This change enables the use of the GeoIP2-Domain database by whitelisting the appropriate field.