Re: [netmod] Reminder: WGLC - draft-ietf-netmod-yang-tree-diagrams

[Robert Wilton <[e-mail]>]

On 15/01/2018 15:26, Martin Bjorklund wrote:
> Robert Wilton <[e-mail]> 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 <[e-mail]> 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.
Yes, agree that it is consistent, but personally I would also be happy 
for the nodes to be intended 2 spaces for the main tree, and then 4 
spaces for everything else (effectively 2 spaces beyond the 
augment/rpc/etc).

>
>>   - 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 think that compact is better, if not quite so pretty, so this solution 
is also OK with me.

Personally, I quite like a relative indent, i.e. the tree is intended 2 
spaces more than its parent "thing", e.g.

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!
Ah, yes, OK.  I was assuming that the tree diagram would always be done 
from the augmenting module, but clearly that is not necessarily the case.


>
>>> 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.
That is fine with me, really just raising so that the point could be 
discussed and closed.

Thanks,
Rob


>
>
> /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
>>>>> [e-mail]
>>>>> https://www.ietf.org/mailman/listinfo/netmod
>>>>> .
>>>>>
>>> .
>>>
> .
>