Skip to content

plugin/geoip: Add support for subdivisions#7728

Merged
snebel29 merged 1 commit into
coredns:masterfrom
fr6nco:pr/7530
Dec 2, 2025
Merged

plugin/geoip: Add support for subdivisions#7728
snebel29 merged 1 commit into
coredns:masterfrom
fr6nco:pr/7530

Conversation

@fr6nco

@fr6nco fr6nco commented Nov 27, 2025

Copy link
Copy Markdown
Contributor

1. Why is this pull request needed and what does it do?

2. Which issues (if any) are related?

3. Which documentation changes (if any) need to be made?

4. Does this introduce a backward incompatible change or deprecation?

@fr6nco

fr6nco commented Nov 27, 2025

Copy link
Copy Markdown
Contributor Author

There was a previous PR #7530, I made the requested changes. I'm in need of this feature too.

@fr6nco fr6nco changed the title Pr/7530 - Fixes Pr/7530 - plugin/geoip: Add support for subdivisions Nov 27, 2025
@fr6nco fr6nco changed the title Pr/7530 - plugin/geoip: Add support for subdivisions plugin/geoip: Add support for subdivisions (Pr/7530) Nov 27, 2025
@snebel29 snebel29 self-assigned this Nov 28, 2025
snebel29
snebel29 reviewed Nov 28, 2025
View reviewed changes
Comment thread plugin/geoip/README.md Outdated
@snebel29

snebel29 commented Nov 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Also, can you (rebase as needed) squash all commits and use a commit message that is consistent with the project commit log history?

I am happy to approve and merge after that.

Cheers.

This was referenced Nov 28, 2025
Closed
Open
@snebel29

snebel29 commented Nov 29, 2025

Copy link
Copy Markdown
Collaborator

I just created #7731 (comment) to look to the unrelated check failing.

@fr6nco fr6nco force-pushed the pr/7530 branch 2 times, most recently from c9a67d2 to 57154c6 Compare November 29, 2025 20:49
@fr6nco fr6nco changed the title plugin/geoip: Add support for subdivisions (Pr/7530) plugin/geoip: Add support for subdivisions Nov 29, 2025
@snebel29

snebel29 commented Nov 30, 2025

Copy link
Copy Markdown
Collaborator

@fr6nco I just merged another geoip contribution in #7730 which caused some conflicts, can you resolve them and rebase?

Let's see if kubernetes-tests check fails again.

@fr6nco

fr6nco commented Nov 30, 2025

Copy link
Copy Markdown
Contributor Author

@snebel29 rebased, tests did finish fine seem like. Thanks

snebel29
snebel29 requested changes Nov 30, 2025
View reviewed changes
Comment thread plugin/geoip/geoip_test.go Outdated
@fr6nco

fr6nco commented Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

wtf, ok please ignore this, somethign went wrong when rebaseing from master

@fr6nco

fr6nco commented Dec 1, 2025

Copy link
Copy Markdown
Contributor Author

Ok sorry for adding a bunch of people to the reviewers. My rebase from master went wrong switching between origin and upstream. Fixed now.

snebel29
snebel29 requested changes Dec 1, 2025
View reviewed changes
Comment thread plugin/geoip/city.go Outdated
@fr6nco fr6nco force-pushed the pr/7530 branch 3 times, most recently from 0d27d6f to 3594426 Compare December 2, 2025 08:50
@snebel29 snebel29 self-requested a review December 2, 2025 10:24
@snebel29

snebel29 commented Dec 2, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

@fr6nco I have edited city.go from github UI, to add a comment to the subdivisions portion. Unfortunately th UI did commit without adding my signature. Do you mind pulling that commit locally, incorporating the comment then squashing it into your original commit?

That should remove the DCO check failing.

I am happy to merge after that.

Cheers.

@fr6nco
plugin/geoip - Support for ISO 3166-2 subdivisions.
0c29b7c 
Metadata `geoip/subdivisions/code` now available if geoip plugin is used.

Signed-off-by: Tomas Boros <tomas.boros92@gmail.com>
@fr6nco

fr6nco commented Dec 2, 2025

Copy link
Copy Markdown
Contributor Author

@snebel29 rebased

snebel29
snebel29 approved these changes Dec 2, 2025
View reviewed changes

@snebel29 snebel29 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@snebel29 snebel29 merged commit b472d3d into coredns:master Dec 2, 2025
11 checks passed
@BrewTestBot BrewTestBot mentioned this pull request Dec 10, 2025
Merged
@SAY-5 SAY-5 mentioned this pull request Apr 21, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@snebel29 snebel29 snebel29 approved these changes

@miekg miekg Awaiting requested review from miekg miekg is a code owner

@bradbeam bradbeam Awaiting requested review from bradbeam

@chrisohaver chrisohaver Awaiting requested review from chrisohaver

@johnbelamaric johnbelamaric Awaiting requested review from johnbelamaric

@rajansandeep rajansandeep Awaiting requested review from rajansandeep

@yongtang yongtang Awaiting requested review from yongtang

@zouyee zouyee Awaiting requested review from zouyee

@dilyevsky dilyevsky Awaiting requested review from dilyevsky

@jameshartig jameshartig Awaiting requested review from jameshartig

@greenpau greenpau Awaiting requested review from greenpau

@Isolus Isolus Awaiting requested review from Isolus

@pmoroney pmoroney Awaiting requested review from pmoroney

@stp-ip stp-ip Awaiting requested review from stp-ip

@SuperQ SuperQ Awaiting requested review from SuperQ

@Tantalor93 Tantalor93 Awaiting requested review from Tantalor93

@thevilledev thevilledev Awaiting requested review from thevilledev

Assignees

@snebel29 snebel29

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@fr6nco @snebel29