Skip to content
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

feat: add nameserver.ListWithParams #9

Merged
merged 3 commits into from
Oct 12, 2023
Merged

Conversation

costasd
Copy link

@costasd costasd commented Oct 12, 2023

Hello,

While trying to use nameserver/List in StackExchange/dnscontrol#2577, we discovered that nameserver.go/List is not following the struct-as-request paradigm that allows for setting all input fields, as most other methods do.

This PR:

  • introduces a new NameserverListRequest, mirroring the List input fields in inwx API
    * Changes List() to use NameserverListRequest for input fields.
    * Fixes a few mostly-internal typos (Namserver* -> Nameserver*)

Thanks!

* Introduce NameserverListRequest struct, mapping to 2.15.10.nameserver.list.
* Convert nameserver/List to use this NameserverListRequest instead.
@ldez
Copy link
Member

ldez commented Oct 12, 2023

Fixes a few mostly-internal typos (Namserver* -> Nameserver*)

sadly you cannot change the name of an exported element because it will break the lib API.

It's the same thing for the method signature.

@ldez ldez added the enhancement New feature or request label Oct 12, 2023
@ldez ldez changed the title nameserver/List: Allow providing all input fields feat: add nameserver.ListWithParams Oct 12, 2023
Copy link
Member

@ldez ldez left a comment

Choose a reason for hiding this comment

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

LGTM (with my modifications)

@ldez ldez merged commit 2dbc0cc into nrdcg:master Oct 12, 2023
7 checks passed
@costasd
Copy link
Author

costasd commented Oct 12, 2023

thanks and makes sense.

@ldez
Copy link
Member

ldez commented Oct 12, 2023

I created a new tag v0.9.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants