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”?
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”.
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
