Re: [lp-wan] Yangdoctors early review of draft-ietf-lpwan-schc-compound-ack-14

[Sergio Aguilar Romero <[e-mail]>]

Hello,

Thanks for your review.

We have published a new version (-16).

Some comments inline.

Best regards,

Authors Compound ACK draft


> On Mar 20, 2023, at 4:57 PM, Ebben Aries via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Ebben Aries
> Review result: On the Right Track
> 
> 1 module in this draft:
> - [e-mail]
> 
> YANG compiler errors or warnings (pyang 2.5.3, yanglint 2.1.55, yangson 1.4.16)
> - No compiler errors or warnings for tree outputs
> - Instance data however fails validation (see below)
> 
> Module [e-mail]
> - Overall, the module is small (2 leaf augments to ietf-schc) and concise.
>  There are some style nits (alignment/spacing) that can be cleaned up by
>  running the module through a linter and adding back to the draft code blocks

The revision performed to version -14 by Robert Wilton included the pyang output that was copied into version -15.


> - As mentioned above, instance data will fail to validate due to the following
>  when stmt
> 
>    when "derived-from(../schc:fragmentation-mode,"
>    +" 'schc:fragmentation-mode-ack-on-error')";
> 
>  Should rather be:
> 
>    when "derived-from-or-self(../schc:fragmentation-mode," +
>         "'schc:fragmentation-mode-ack-on-error')";
> - Since you are applying the same when restriction to 2 leaf nodes here, I
>  would recommend grouping the leaf nodes and gating the augment by way of
>  uses + when
> 
>  e.g.
> 
>  augment "/schc:schc/schc:rule/schc:nature/schc:fragmentation" +
>          "/schc:mode/schc:ack-on-error" {
>    description
>      "";
> 
>    uses ack-on-error {
>      when "derived-from-or-self(./schc:fragmentation-mode, " +
>           "'schc:fragmentation-mode-ack-on-error')";
>    }
>  }
> 

We have fixed the “+” location. 



> General comments on the draft/modules:
> - Section 5.2 - It appears that only 1 augment is conveyed here and the Figure
>  10 line is out of place.  This section should conform to RFC8340 and be
>  labeled "Tree Diagram" as seen in other published drafts/RFCs

We have modified Figure 10 label.


> - Module contact information - Feel free to include authors as stated in
>  RFC8407 Section 4.8

One more author was added.

> - Module description - Put the description of the module at the top and the
>  Copyright information beneth with correct line breaks and removal of the
>  asterisk delimeter

Module description was move to the top and asterisk have been removed. When the new version was uploaded, we have a warning saying that the IETF Trust Copyright statement seems to be missing, but it is there.

> - For all description statements in the module, use correct capitilization and
>  be as descriptive as possible without being overly verbose.

Some description was added.

> - Section 7 - Include the full Security Considerations as is in any other
>  drafts/RFCs related to YANG modules vs. point to a related RFC

The draft mentions that same security considerations defined in RFC8724  applied. We added RFC9363 to the security considerations.

> - Section 8 - IANA Considerations.  You are introducing a new module that
>  contains a new namespace so will require registering a new URI.  This will
>  need to follow the same process and contain the same verbiage as other
>  related drafts/RFCs
> 

We understand that this document has no IANA actions.


> Example validated instance data after the when stmt fix:
> 
> <schc xmlns="urn:ietf:params:xml:ns:yang:ietf-schc"
>  xmlns:schc-compound-ack="urn:ietf:params:xml:ns:yang:ietf-lpwan-schc-compound-ack">
>  <rule>
>    <rule-id-value>100</rule-id-value>
>    <rule-id-length>1</rule-id-length>
>    <rule-nature>nature-fragmentation</rule-nature>
>    <fragmentation-mode>fragmentation-mode-ack-on-error</fragmentation-mode>
>    <direction>di-up</direction>
>    <fcn-size>2</fcn-size>
>    <schc-compound-ack:bitmap-format>schc-compound-ack:bitmap-compound-ack</schc-compound-ack:bitmap-format>
>    <schc-compound-ack:last-bitmap-compression>true</schc-compound-ack:last-bitmap-compression>
>  </rule>
> </schc>
> 
> {
>    "ietf-schc:schc": {
>        "rule": [
>            {
>                "rule-id-value": 100,
>                "rule-id-length": 1,
>                "rule-nature": "ietf-schc:nature-fragmentation",
>                "fragmentation-mode":
>                "ietf-schc:fragmentation-mode-ack-on-error", "direction":
>                "ietf-schc:di-up", "fcn-size": 2,
>                "ietf-lpwan-schc-compound-ack:bitmap-format":
>                "ietf-lpwan-schc-compound-ack:bitmap-compound-ack",
>                "ietf-lpwan-schc-compound-ack:last-bitmap-compression": true
>            }
>        ]
>    }
> }
> 

Great. 

>