Reviewer: Joe Clarke Review result: Not Ready I have been tasked to perform a LC review on this draft on behalf of YANG Doctors. This document defines a YANG module to codify the rules for Static Context Header Compression. While I say that it is "Not Ready" from a LC perspective, it may be more ready than that lets on. I have a few questions of the authors to gain perspective on what they're trying to achieve. I have also found a number of issues in the module itself. See below: - YANG tree has subtle differences from the YANG module; regenerate - Run the module through `pyang -f yang` to generate a canonical formatted version - No linting errors, though the copyright needs to be updated - Pet peeve: I prefer descriptions to begin with a capital letter and end with a full-stop '.' - Many descriptions (especially of identities) have an RFC in them. Please make this a reference as well. - For fl-type, if this is a positive integer, why not use an unsigned type? - appiid and deviid should be AppIID and DevIID as stated in RFC8724 - In grouping tv-struct's description: s/enconding/encoding/ - Leaf "index" in tv-struct grouping: s/indicia/index/ - In leaf field-position: s/occurence/occurrence/ - You use "YANG referenceid" a few times, but this isn't a thing per se. You tend to use this to mean an identity reference. In all cases, though, I think it would be best to more clearly state what the leaf is/does and leave out that fact that it uses an identityref - In grouping compression-rule-entry: s/identifer/identifier/, but again, I'd leave out these YANG bits from descriptions. - In your tv-struct grouping, define "index" as the first leaf. That seems a bit more logical to me. - For leaf comp-decomp-action, your description is a tautology (which is another pet peeve of mine). Can you sweep descriptions and make sure they provide some additional context or at least a reference? - For something like di-type, does it make sense to be an identity? Seems like this could be an enumeration as I don't think you'll have directions other than up, down, bi? But maybe you think these types may be extended? Just curious. - In leaf direction: s/bidirectionnal/bidirectional/ and s/forbiden/forbidden/ - For w-size, why not use derived-from-or-self here (same in other places where fragmentation-mode is referenced)? - In grouping compression-content: s/identifed/identified/ - See Section 3.4 of https://datatracker.ietf.org/doc/html/rfc8407 on how to reference YANG tree diagrams - Why do you have empty cases for the mode choice (e.g., no-ack, ack-always)? It's not clear you need a choice here. And if you need it, you could just have one "when" clause on the case itself. - There is another empty case for the no-compression case in the nature choice. The description there says that a rule is required. - Can one have both features for compression and fragmentation? The choice seems to imply no, but I am curious. I didn't get the impression that they were mutually exclusive from 8724. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
