Re: [secdir] Secdir last call review of draft-ietf-netconf-notification-capabilities-17

[Benoit Claise <[e-mail]>]

Hi Barry,

Thanks for your insightful review.
All remarks improve (the reading of) the specifications.
See inline for some some specific remarks.

On 10/1/2021 5:15 PM, Barry Leiba via Datatracker wrote:
> Reviewer: Barry Leiba
> Review result: Has Nits
>
> Well written and easy to read; thanks.  I only have some very minor editorial
> suggestions that I ask you to consider:
>
> — Section 1 —
>
>     Many such capabilities are
>     specific to either the complete system, individual YANG datastores
>     [RFC8342], specific parts of the YANG schema, or even individual data
>     nodes.
>
> Nit: “either” is correctly used for two items (“either A or B”).  For the four
> items here, you might just eliminate the word “either”, as it’s not really
> needed.
>
>     A NMS implementation that wants to
>     support notifications, needs the information about a system's
>     capability to send "on-change" notifications.
>
> I often find that I have to read this sort of thing (“A needs B to do C”) twice
> to determine whether you mean that A requires that B do C, or that A needs B so
> that A can do C — it’s ambiguous, so it requires extra analysis by the reader.
> I suggest the following (which also eliminates the personification of NMS):
>
> NEW
>     An NMS implementation that supports
>     notifications needs the information about a system's
>     capability so it can send "on-change" notifications.
> END
>
> — Section 2 —
>
>     This allows a user to
>     discover capabilities both at implementation-time and run-time.
>
> Nit: The “at” is factored wrong with respect to “both”. Either “both at
> implementation time and at run time” or “at both implementation time and run
> time”.  In either case, no hyphens here, as they’re not compound modifiers.
>
>        The file MUST be
>        available already at implementation-time retrievable in a way that
>        does not depend on a live network node.
>
> Nit: No hyphen (again, not a modifier), and it needs a comma after it:
> “implementation time,”
>
>        For the run-time use-case
>
> Nit: Here, “run-time” is a modifier and needs the hyphen, but “use case” is a
> noun and does not.
>
>        (implementing the publisher) during run-time.  Implementations
>        that support changing these capabilities at run-time SHOULD
>
> Nit: No hyphens in “run time” for these two (nouns, not modifiers).
>
> — Section 3 —
>
>     A specific case is the need to specify capabilities is the YANG-Push
>     functionality.
>
> I’m not sure of the right fix for this, but the two instances of “is” can’t
> both be right.  Maybe the first should be “of”?

A specific case is the need to specify capabilities in the YANG-Push
    functionality.

>
>     As defined in [RFC8641] a publisher may allow
>     subscribers to subscribe to updates from a datastore and subsequently
>     push such update notifications to the receiver.
>
> It’s unclear who is pushing: it looks like it could be the subscribers.  Maybe
> clarify this way?:
>
> NEW
>     As defined in [RFC8641] a publisher may allow
>     subscribers to subscribe to updates from a datastore and will
>     subsequently push such update notifications to the subscriber.
> END
Yes to the above.
>
>     unless the subscriber has some means to
>     identify which objects "on-change" notifications are supported.
>
> Missing word: “are supported for.”
>
> — Section 4 —
>
>     It SHOULD be used by other modules to augment-in specific
>     capability information.
>
> The term “augment-in” is not one I’m familiar with.  If it’s common in YANG,
> that’s fine.  If not, maybe rephrase?

    It SHOULD be used by other modules to augment in specific
    capability information.


>
>     data is considered as if it was part
>     of the running datastore.
>
> Ultra-nit: “as if it were part”: subjunctive mood is needed after “as if”.
>
>
> .
Thanks again.

Regards, Benoit