Thanks for the review Martin. See my responses inline... On Tue, Sep 03, 2024 at 07:33:39AM -0700, Martin Björklund via Datatracker wrote: > Reviewer: Martin Björklund > Review result: Ready with Nits > > Here is my YANG doctor's review of draft-ietf-netmod-rfc6991-bis-16. > > o typedef email-address > > The domain part of "email-address" is different from the type > "domain-name". This looks a bit odd. If special characters can > occur in the domain part of an email address, one would assume that > they can occur in a domain name as well. Email addresses are more complex than just <label@domain-name>. We meanwhile also received a review suggesting that we support internationalized email addresses and this likely means u-labels in the domain name part instead of a-labels we use in the domain-name type. As a consequence, I believe that capturing all details of valid email addresses precisely in a regular expression is a flawed idea as this leads to super complex regular expressions that are hard to get right (for the interested readers, check our "How can I validate an email address using a regular expression?" on stack overflow. So I will revert to a rather course grained regular expression. I will share more details in my response to the Artart review. > o typedef protocol-number > > "The protocol-number type represents an 8-bit Internet > protocol number, carried in the 'protocol' field of the > IPv4 header or in the 'next header' field of the IPv6 > header. If IPv6 extension headers are present, then the > protocol number type represents the upper layer protocol > number, i.e., the number of the last next header' field > ^^^ ' missing > of the IPv6 extension headers. Fixed in my sources. > o typedef ipv6-address-and-prefix > > "The ipv6-address-and-prefix type represents an IPv6 > address and an associated ipv4 prefix. > > s/ipv4 prefix/IPv6 prefix/ Fixed in my sources. > o typedef ipv4-address-and-prefix > > "The ipv4-address-and-prefix type represents an IPv4 > address and an associated ipv4 prefix. > > s/ipv4 prefix/IPv4 prefix/ Fixed in my sources. > o "schema node instance" > > This term is used in a few places in ietf-yang-types, for example: > > A schema node instance of this type will be set to zero (0) > on creation > > This isn't correct, since a schema node is a node in the schema > tree, and doesn't have a value. With RFC 7950 terminology, it would > be "a node in the data tree". It is unfortunate that there is no > specific term for this in RFC 7950. > > Perhaps it would be easier to just write "An instance of this > type...". > > (I know that this was not correct RFC 6991 either) I suggest we use 'A data tree node using this type' as a replacement of "A schema node instance of this type'. Since a data tree is defined in RFC 7950 to be an instantiated tree of any data modeled with YANG, this should be fix the problem. /js -- Jürgen Schönwälder Constructor University Bremen gGmbH Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
