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