Robert Wilton <rwil...@cisco.com> wrote: > Hi Martin, > > All OK with me except where I have further comments/questions inline > below: > > On 15/01/2018 14:39, Martin Bjorklund wrote: > > Hi, > > > > Thanks for the review! Comments inline. > > > > Robert Wilton <rwil...@cisco.com> wrote: > >> Hi Lou, Martin > >> > >> I've done a quick review of draft -04. > >> > >> It looks well written to me. > >> > >> I have a spotted a few minor nits/questions. > >> > >> Section 1: > >> > >> (i) "Such diagrams are commonly used to provided " => "Such diagrams > >> are used to provide"? > > Ok. > > > >> (ii) "This document provides the syntax used in YANG Tree Diagrams." > >> => "This document describes the syntax used in YANG Tree Diagrams", or > >> if not "describes", perhaps "specifies"? > > I changed to "describes". > > > >> (iii) "common practice is include" => "common practice is to include" > > Ok. > > > >> Section 2: > >> (iv) Are the top level data nodes really offset by 4 spaces, or should > >> this be 2 spaces? The example in section 2, and section 4 seem to > >> differ here. The submodule example in Sec 2.1 looks like it is using > >> 2 spaces. > > It should be 4 spaces. I fixed the example in 2.1. > Hum, OK. Is this the right choice? > - It means that the tree is indented 2 spaces further than everything > else (e.g. groupings, augments, etc).
Well, I think it is consistent as it is now: module: <module-name> +--<node> | +--<node> | +--<node> +--<node> +--<node> +--<node> augment <target-node>: +--<node> +--<node> +--<node> +--<node> Nodes are always intended 4 spaces. > - YANG modules in RFC's already struggle with line length anyway, > hence wouldn't the use of 2 spaces give the model a bit more space. This is a good argument. I'm happy to save spaces by using 2 instead: module: <module-name> +--<node> | +--<node> | +--<node> +--<node> +--<node> +--<node> augment <target-node>: +--<node> +--<node> +--<node> +--<node> > I also think that it would be good to check the indent of all the tree > diagram snippets in the doc, it looks like they may be using somewhat > varying levels of indents (between 2 and 6 spaces). Ok. > >> (v) "is prefaces with" => "is prefaced with" > > Ok. > > > >> (vi) Section 2.2, should there be an example of an unexpanded uses > >> statement? I was wondering if this section was under specified? > > I have added: > > > > For example, the following diagram shows the "tls-transport" grouping > > from [RFC7407] unexpanded: > > > > +--rw tls > > +---u tls-transport > > > > If the grouping is expanded, it could be printed as: > > > > +--rw tls > > +--rw port? inet:port-number > > +--rw client-fingerprint? x509c2n:tls-fingerprint > > +--rw server-fingerprint? x509c2n:tls-fingerprint > > +--rw server-identity? snmp:admin-string > Yes, looks good. > > > > >> Section 2.6: > >> (vii) "If the node is augmented into the tree from another module, its > >> name is printed as <prefix>:<name>." Does there need to be a > >> clarification that the prefix name used is the one used by the > >> module's import statement? Or does it use the prefix statement from > >> the imported modules instead (I know that these should normally be the > >> same, but this is not guaranteed). > > Since this is used when a node is augmented *into* the main tree, it > > can't be the prefix in import, since the augmenting module is not > > imported from the augmented module. I did: > But the YANG module must import the module that it is augmenting. If a > local import prefix is used in the actual YANG module then it would be > slightly harder to relate the tree output back to local import > prefixes used in the YANG module. Here's an example: module: ietf-interfaces +--rw interfaces | +--rw interface* [name] | +--rw name string ... | +--rw ip:ipv4! ietf-interfaces doesn't import ietf-ip, so there is no other prefix to use for "ipv4" than the one defined in ietf-ip! > > OLD: > > > > If the node is augmented into the tree from another module, > > its name is printed as <prefix>:<name>. > > > > NEW: > > > > If the node is augmented into the tree from another module, > > its name is printed as <prefix>:<name>, where <prefix> is the > > prefix defined in the module where the node is defined. > This is OK with me, but there is still a potential for a prefix > namespace clash (since prefixes are not guaranteed to be unique). > > An alternative solution would be for the YANG tree diagram to list (at > the beginning or the end of the diagram) the mappings from prefix -> > module name used. The tree diagram is supposed to give a simplified overview of the structure of a module. I agree with your statemenet below that such a mappig adds noise... I prefer to keep the diagram as is. /martin > This has the bonus that it also explicitly lists > the YANG modules that are being augmented, but conversely, this might > end up just adding unnecessary noise to a diagram that should be short > and simple ... > > A second alternative would be to add some vague text to state that the > prefix to module mapping should be explicitly listed in any cases > where the prefix name alone is not obvious. > > Thanks, > Rob > > > > > >> Section 3.2. > >> (viii) It would be slightly easier to read if there wasn't a linebreak > >> between "--" and "tree-print-groupings", not sure if that is feasible > >> to force this. > > I don't think I can enforce this, but I'll look into it. If nothing > > else, the RFC editor will fix this. > > > > > > /martin > > > > > >> Thanks, > >> Rob > >> > >> On 10/01/2018 16:16, joel jaeggli wrote: > >>> Just a reminder given the date that this was posted. This last call is > >>> expected to complete Monday 1/15/18. > >>> > >>> Thanks > >>> > >>> joel > >>> > >>> > >>> On 1/1/18 2:01 PM, joel jaeggli wrote: > >>>> Greetings, > >>>> > >>>> We hope the new year is a time to make great progess on outstanding > >>>> documents before preparation for the London IETF begins in earnest. > >>>> > >>>> This starts a two-week working group last call on: > >>>> > >>>> YANG Tree Diagrams > >>>> draft-ietf-netmod-yang-tree-diagrams > >>>> > >>>> https://datatracker.ietf.org/doc/draft-ietf-netmod-yang-tree-diagrams/ > >>>> > >>>> Please send email to the list indicating your support or concerns. > >>>> > >>>> We are particularly interested in statements of the form: > >>>> > >>>> * I have reviewed this draft and found no issues. > >>>> * I have reviewed this draft and found the following issues: ... > >>>> > >>>> > >>>> Thank you, > >>>> NETMOD WG Chairs > >>>> > >>>> > >>>> > >>>> > >>> _______________________________________________ > >>> netmod mailing list > >>> netmod@ietf.org > >>> https://www.ietf.org/mailman/listinfo/netmod > >>> . > >>> > > . > > > _______________________________________________ netmod mailing list netmod@ietf.org https://www.ietf.org/mailman/listinfo/netmod