Early Review of draft-ietf-dots-rfc8782-bis-00
review-ietf-dots-rfc8782-bis-00-yangdoctors-early-aries-2020-09-23-00

2 modules in this draft:
- ietf-dots-signal-channel@2020-07-02.yang
- iana-dots-signal-channel@2020-05-28.yang

YANG compiler errors or warnings (pyang 2.3.2, yanglint 1.9.2)
- No compiler errors or warnings however pyang 2.3.2 is currently the only
  compiler verified to emit YANG sx:structure data in tree format.  Instance
  data could not yet easily be validated given the current linters/compilers.

General comments on the draft/modules:
----------------------------------------
The modules defined in this draft are an update replacement to those defined
in RFC8782 with only 1 being updated from the prior publication.
  - ietf-dots-signal-channel@2020-05-28.yang (updated)
  - iana-dots-signal-channel@2020-05-28.yang

- First, I share some of the same comments as the previous review in that it
  takes some additional thought on modeling structures for alternate protocol
  communications with the YANG language.  Overall, I think this is likely fine
  but also question where the draft/RFC specification outlines rules that are
  not conveyed within the data-model.  Some examples include specifying
  mandatory attributes but not making use of the YANG 'mandatory' statement.
  Other cases are stating the intent of reserved or invalid integers but not
  putting the same restrictions on the types under a given data node.  Overall
  this means that you cannot validate the instance/payload data according to
  the data-model alone.

- Previous review suggested the main edits required switching the top-level
  container 'dots-signal'.  This has now been fulfilled by that of an
  sx:structure in this revision.

- A recent discussion among YANG doctors suggests the use of normative
  language (RFC2119) in YANG description statements. For example 'should' =>
  'SHOULD', 'must' => 'MUST', etc... A benefit to this approach is RFC
  comprehension even when the module is stripped from the source document it
  is contained under.

- (draft) L887 nit: "As a reminder, the prefix length must be less than or
  equal to 32 (for IPv4) for 128 (for IPv6)"

Module ietf-dots-signal-channel:
----------------------------------------
- IMO, the module prefix 'signal' is too generic for what is specific to
  message payload over a protocol that uses CoAP.  Looking at all the
  published RFCs for dots, we have the same use of generic prefixes.  Since
  the prefix is of local scope, it is not an entirely large issue but in
  hindsight should have had some better consistency especially when the
  recommendation to module importers is to utilize the defined prefix of said
  module.

- This module imports and references 'ietf-dots-data-channel' as prefix
  'ietf-data'.  Not only is this too generic as I mentioned above, it is not
  using the suggested prefix of 'data-channel' (which is also too generic) as
  defined within that module (RFC7950 Section 7.1.4)

   "When used inside the "module" statement, the "prefix" statement
    defines the prefix suggested to be used when this module is imported."

- Minor nit: It appears all author email addresses have been updated since the
  RFC8782 variant replacing '@' with '&'

Module iana-dots-signal-channel:
----------------------------------------
- Similar to what I mention above regarding the module prefix, we use a
  generic 'iana-signal' prefix here which is inconsistent and not descriptive
  of the overall intent of the module.

Overall, I'm not sure if the other referenced and published DOTS YANG modules
went through a YD review but the comments above really should be addressed in
a new revision of those modules.  Once the above is addressed and possibly
some discussion around when type restrictions are not conveyed within the
model, it is probably on the right track.  Since this is not the usual use of
YANG in the context of YD reviews, it is a bit of an on the fly analysis which
may require some additional discussion among other YANG doctors.