Last Call Review of draft-ietf-netmod-entity-07
review-ietf-netmod-entity-07-yangdoctors-lc-krejci-2018-01-04-00

The document itself and normative parts seem fine to me, the only issue I see
is with the ietf-hardware-state module in non-normative appendix A. It seems to
me that this temporary non-NMDA module does not conform to its purpose as
described in RFC6087bis. According to guidelines, such a module is intended to
provide state (config false) data in case the server does not implement NMDA
(to bridge the time period until NMDA is implemented). So such a server is IMHO
intended to implement both modules, foo and foo-state. The foo-state contains
"the top-level config-false data nodes that would have been defined in a legacy
YANG module" - so it's only the ro mirror of data nodes. But
ietf-hardware-state contains notifications, which are not the data nodes as
defined in RFC7950. I understand why the notifications were placed also into
the ietf-hardware-state - the module's description states that "If a server
that implements this module but doesn't support NMDA also supports
configuration of hardware components, it SHOULD also implement the module
'ietf-hardware' ...", so it allows its standalone usage in case the server does
not support hw configuration. But in such a case, the server can implement
ietf-hardware with deviations on the config=true nodes. The same way it had to
implement the legacy YANG module (before NMDA).

So I think that the notifications should be removed from ietf-hardware-state
and the module's description should change this way:

OLD

  If a server that implements this module but doesn't support NMDA
  also supports configuration of hardware components, it SHOULD
  also implement the module 'ietf-hardware' in the configuration
  datastores. The corresponding state data is found in the
  '/hw-state:hardware' subtree.

NEW

  If a server that implements this module but doesn't support NMDA,
  it MUST also implement the module 'ietf-hardware' in the
  configuration datastores. The corresponding state data is found
  in the '/hw-state:hardware' subtree.