Skip to main content

Last Call Review of draft-ietf-opsawg-vpn-common-06
review-ietf-opsawg-vpn-common-06-yangdoctors-lc-krejci-2021-03-28-00

Request Review of draft-ietf-opsawg-vpn-common
Requested revision No specific revision (document currently at 12)
Type Last Call Review
Team YANG Doctors (yangdoctors)
Deadline 2021-04-05
Requested 2021-03-22
Requested by Joe Clarke
Authors Samier Barguil , Oscar Gonzalez de Dios , Mohamed Boucadair , Qin Wu
I-D last updated 2021-03-28
Completed reviews Yangdoctors Early review of -02 by Radek Krejčí (diff)
Rtgdir Last Call review of -06 by Ron Bonica (diff)
Tsvart Last Call review of -06 by Wesley Eddy (diff)
Yangdoctors Last Call review of -06 by Radek Krejčí (diff)
Tsvart Last Call review of -09 by Wesley Eddy (diff)
Rtgdir Last Call review of -09 by Victoria Pritchard (diff)
Opsdir Last Call review of -09 by Tim Wicinski (diff)
Intdir Last Call review of -09 by Suresh Krishnan (diff)
Genart Last Call review of -09 by Joel M. Halpern (diff)
Assignment Reviewer Radek Krejčí
State Completed
Request Last Call review on draft-ietf-opsawg-vpn-common by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/WZJU8-tPAQlUeeIKZzDUBwNhUy0
Reviewed revision 06 (document currently at 12)
Result Ready w/nits
Completed 2021-03-28
review-ietf-opsawg-vpn-common-06-yangdoctors-lc-krejci-2021-03-28-00
The I-D document contains a single YANG module ietf-vpn-common.

The I-D as well as the module itself are in a good shape, I have just two notes:

* module's description
This is my fault since I came with it in the previous review. There should be
really `Section 4.c` as you had. What confused me is how the RFC shows it in
its htmlized version, where the link is connected just with the 'Section 4'
string and pointing to the RFC itself instead of actually referenced Trust
legal document.

* `service-status` grouping
While the `status-timestamp` grouping was modified since my previous review,
the `service-status` still contains only the service-status/status/oper-status
container with the config false flag. My arguments are still the same, the
description of the other items there says that they are status information, so
they should be specified that way. The uses statement doesn't have its own
config statement, so if you want to place the mentioned groupings into config
true data, an extra grouping or refine will be required.