[yang-doctors] Yangdoctors last call review of draft-ietf-rift-yang-11

[Michal Vaško via Datatracker <[e-mail]>]

Reviewer: Michal Vaško
Review result: On the Right Track

This is my second YANG doctors review of draft-ietf-rift-yang. As mentioned in
my previous review, the short descriptions and often missing references make
the module difficult to understand and should be added.

typedef 'level':
- Generic name, short description, and no reference makes this a vague typedef,
ideally all 3 should be improved/added.

grouping 'security', leaf 'security-type':
- Descriptions of enum values reference the RFC text, which is not suitable.
When the YANG module is used on its own, the value need to be explained in the
YANG module. Ideally a reference added as well.

grouping 'security', case 'auth-key-explicit':
- The leaf 'key-id' seems completely redundant, its purpose is unclear.
- The leaves 'key' and 'crypto-algorithm' seem like an oversimplified way of
storing keys but I see that it was inspired by how ietf-key-chain stores them
so please add a reference to that RFC.

grouping 'neighbor-node':
- Description mentions bits, which can be specified as units directly.

grouping 'neighbor':
- Leaf 'removal-reason' seems tied to 'removed-from-consideration' but this is
not expressed in YANG. A must condition or when condition with mandatory flag
can be used. - Leaf 'bfd-up' should probably have a default value. Also, may be
better to rename it to 'bfd' and use an enumeration with enums 'up' and 'down'.

grouping 'tie-header':
- Leaf 'origination-lifetime' and 'remaining-lifetime' should have units
specified.

augment "/rt:routing/rt:control-plane-protocols/rt:control-plane-protocol":
- Leaf 'configured-level' has a complex description that should be expressed in
YANG instead. - Leaf 'state' enum values are using a different naming scheme.