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

[Benoit Claise <[e-mail]>]

Barry, all,

In order to help the IESG with the telechat review for this coming 
Thursday, I posted version v18
https://datatracker.ietf.org/doc/draft-ietf-netconf-notification-capabilities/

Thanks, Benoit
On 10/4/2021 2:28 PM, Barry Leiba wrote:
> Thanks, Benoît!
>
> Barry
>
> On Mon, Oct 4, 2021 at 8:16 AM Benoit Claise <[e-mail] 
> <mailto:[e-mail]>> wrote:
>
>     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
>