fix: allow dns record types to be specified without the corresponding value when listing dns records #162
Conversation
| #[serde_with::skip_serializing_none] | ||
| #[derive(Serialize, Clone, Debug)] | ||
| #[serde(tag = "type")] | ||
| pub enum DnsRecordTypeWithOptionalContent { |
There was a problem hiding this comment.
Why do we need the "optional value"?
The ListDnsRecordsParams seems to only really need something as:
pub enum DnsRecordType {
A,
AAAA,
CNAME,
NS,
MX,
TXT,
SRV,
}
There was a problem hiding this comment.
Because it (type) is an optional parameter as shown in the documentation?
There was a problem hiding this comment.
Because it (
type) is an optional parameter as shown in the documentation?
I believe you mean the content parameter instead of type. If a Some value is used in the content field then it is added in addition to the type parameter when serializing the record_type field of ListDnsRecordsParams due to #[serde(flatten)]. It may be cleaner to instead add a separate content field to ListDnsRecordsParams so it can be specified without also specifying type.
There was a problem hiding this comment.
Then we won't be able to ensure the type of content matches with the type of type.
|
Thanks for this fix! I've left a comment/question |
… value when listing dns records
poscat0x04
force-pushed
the
fix-list-dns-records-endpoint
branch
from
1d83928 to
f1a26e8
Compare
|
What can I do to get this merged? |
This pull request fixes the problem described in issue #145 where record types must be specified together with a value of that type by adding a type
DnsRecordTypeWithOptionalContent(I'm not good at naming things).closes #145
Relevant API doc: https://api.cloudflare.com/#dns-records-for-a-zone-list-dns-records