Skip to main content

Last Call Review of draft-ietf-netconf-notification-capabilities-17
review-ietf-netconf-notification-capabilities-17-secdir-lc-leiba-2021-10-01-00

Request Review of draft-ietf-netconf-notification-capabilities
Requested revision No specific revision (document currently at 21)
Type Last Call Review
Team Security Area Directorate (secdir)
Deadline 2021-10-01
Requested 2021-09-17
Authors Balázs Lengyel , Alexander Clemm , Benoît Claise
Draft last updated 2021-10-01
Completed reviews Yangdoctors Last Call review of -05 by Ladislav Lhotka (diff)
Secdir Last Call review of -17 by Barry Leiba (diff)
Assignment Reviewer Barry Leiba
State Completed
Review review-ietf-netconf-notification-capabilities-17-secdir-lc-leiba-2021-10-01
Posted at https://mailarchive.ietf.org/arch/msg/secdir/wL2t_0ngaSWP0tpEHTXspAccEaA
Reviewed revision 17 (document currently at 21)
Result Has Nits
Completed 2021-10-01
review-ietf-netconf-notification-capabilities-17-secdir-lc-leiba-2021-10-01-00
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”?

   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

   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?

   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”.