feat: add support for libdns ZoneLister #9
Conversation
|
Thank you for the PR! |
|
@znkr Is there a chance to reopen and merge this PR? The changes look good to me. Pagination shouldn't be an issue here. This could be implemented in another step for all endpoints that need it. |
There are a number of unresolved comments that haven't been addressed. I am definitely open to continue with this PR, but it does require a little bit of work. |
Am I missing something? The only comment I see on this PR is the one about pagination |
|
It's entirely possible that I am not using PR reviews correctly (I don't use it often). But this is what I see:
Have I missed sending my comments? I checked and didn't find a send button. |
|
The button appeared once I ausgehen |
|
@haylinmoore Thanks a lot for your quick update. 🏎 I did some tests with the application I need |
znkr
left a comment
znkr
left a comment
There was a problem hiding this comment.
This looks good. I have one more comment, but it shouldn't be a lot of work.
| for _, testDomain := range testDomains { | ||
| if !zoneMap[testDomain] { | ||
| t.Errorf("Failed to find %s in ListZones call", testDomain) | ||
| } |
There was a problem hiding this comment.
I believe the intend is to make sure to allow domains that are not part of the test domains to be part of the result, because there can be preexisting domains (good idea, btw). This is not immediately clear from the code (at first I thought this was a bug).
Also I generally prefer using go-cmp for things like this. In this case, I would use something like the (untested) code below:
testDomains := map[string]bool{
// ...
}
// ...
got, err := p.ListZones()
if err != nil {
t.Fatal(err)
}
var want []libdns.Zone
for domain := range testDomains {
want = append(want, libdns.Zone{Name: domain})
}
// We only control a limited number of zones in the test account, there may be preexisting zones that
// are unknown to us. Ignore all unknown zones.
opts := cmp.Options{
cmpopts.IgnoreSliceElements(func(zone libdns.Zone) bool { return !testDomain[zone.Name] }),
comports.SortSlices(func(a, b libdns.Zone) int { return cmp.Compare(a.Name, b.Name) }),
}
if diff := cmp.Diff(want, got, opts); diff != "" {
t.Errorf("ListZones() unexpected diff [-want +got]: %s", diff)
}4 participants
No description provided.