Thank you Per!

Please see my responses in-line....

-snip-


Medium issues:

The security considerations need to be updated and mention RESTCONF as well. 
Suggest to use the Security Considerations template from rfc8407bis.

<scott>
Ok, I will use rfc8407bis as template
</scott>

Excellent partitioning and presentation of sensitive data nodes per YANG module!
<scott>
Thanks I will keep this when I apply the rfc8407 template.
</scott>


Validating the YANG modules

[email protected]:

There are only whitespace and formatting issues with the models, all of which 
are harmless

pyang ok
yanglint ok

pyang -f yang --keep-comments --yang-line-length 69 \
    [email protected] > new.yang \ && diff 
[email protected] new.yang

(Omitting empty line removal differences.)

<scott>
Thanks for this, I have trouble sometimes with "weird spacing".
</scott>


-snip


[email protected]:

There are only whitespace and formatting issues with the models, all of which 
are harmless

pyang ok
yanglint ok

pyang -f yang --keep-comments --yang-line-length 69 \
    [email protected] > new.yang \ && diff 
[email protected] new.yang

(Omitting empty line removal differences.)


<scott>
I'll do the same thing in this module.
</scott>

-snip

Nits:

The year in the copyright stanza for ietf-if-flexible-encapsulation needs to be 
updated.

<scott>
Yes
</scott>


Perhaps Scott Mansfield should be added to the YANG modules as an author/editor?

<scott>
Ok
</scott>


RFC XXXX is referenced in both ietf-if-flexible-encapsulation and 
ietf-if-vlan-encapsulation for ietf-if-extensions. However, in the descriptions 
and revision references it says that ietf-if-flexible-encapsulation is part of 
RFC XXXX.

The YANG module ietf-if-extensions is part of 
I-D.draft-ietf-netmod-intf-ext-yang and should be referenced as such.

<scott>
I will look to clean this up.  The intf-ext-yang is in the same state as 
sub-intf-vlan, so they could be considered together so we don't have to go back 
in and add the RFC numbers.  I'm not sure the best way to handle this, so any 
suggestions are appreciated.
</scott>


The YANG trees don't have they types aligned on column, it is slightly easier 
on the eyes if they are. (When I emitted the trees with pyang, the types were 
aligned; perhaps update the tooling to generate the
trees?)
<scott>
I'll regenerate the trees, good idea.
</scott>


Abstract

orignating -> originating


ietf-if-vlan-encapsulations.yang revision description

encapulations -> encapsulations


Section 10.2 Last paragraph

    are added, modified or deleted.

Oxford comma is used everywhere else in the document, change to

    are added, modified, or deleted.

<scott>
I'll fix these typos!
</scott>


Out of scope for this review but worth mentioning: The use of the "-grouping" 
suffix in grouping identifiers is recommended to be avoided.
The YANG modules in this document does not do this, but the imported IEEE 
802.1Q YANG module does. If, by any chance, there is collaboration on YANG 
modules, or liaisons participating in both IETF and IEEE: Please forward this 
recommendation for future YANG modules. :)

<scott>
I will bring this up with the YANG coordination group in the IEEE 802.1 and 
suggest a guideline.  It might be a while before anything will change however.  
I will also review the latest version of rfc8407bis again, good to stay on top 
of that.
</scott>


Thank you very much for your review.

Regards,
-scott.


--
Per


_______________________________________________
netmod mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to