Subject: [yang-doctors] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15

From: Acee Lindem <[email-masked]>
Date: Tue, 13 Nov 2018 13:26:08 -0800

Reviewer: Acee Lindem
Review result: On the Right Track

I have reviewed this document as part of the YANG doctors directorate's 
ongoing effort to review all IETF documents being processed by the IESG.  These 
comments were written with the intent of improving the operational aspects of the 
IETF drafts. Comments that are not addressed in last call may be included in AD reviews 
during the IESG review.  Document editors and WG chairs should treat these comments 
just like any other last call comments. 

Summary: This document includes the augmentations to the ietf-te-topology model for
         WSON in the model ietf-wson-topology.yang and the new Optical Layer-0 types
         in ietf-layer0-types.yang. I'm not a Subject Matter Expert (SME) in this 
         area so my comments are mainly related to YANG and the draft. 

         While the information content of this draft is sound, there are a number 
         of minor issues that need to be resolved. Therefore I would characterise the
         review state as "On the Right Track". 

Major Issues: None

Minor Issues: 

      1. The document has 7 authors. This will have to be ok'ed by the 
         responsible AD. 
      2. There is no consistency in the usage of periods in the YANG descriptions. 
      3. The type descriptions in ietf-layer0-types are rather terse and many times
         just repeat part of the leaf name, e.g., "Section.".  Also, optical domain 
         abbreviations are not expanded.
      4. The indentation in ietf-layer0-types is not consistent. I tried to fix. 
      5. When conditions starting with "Augement maximum LSP bandwidth of TE
         link template" and ending with "Augment unreserved bandwidth of TE
         link template" in ietf-layer0-types are commented out.
      6. If the model ietf-wson-topology.yang is rendered, the 
         "/* .... */" comments would be better served being in the augmentation
          descriptions rather than in comments. Most of augmentation descriptions
          currently say "WSON label.". 
      5. When conditions starting with "Augment label hop of underlay primary path
         of TE link template" and ending "Augment label restrictions end of TE 
         link template" in ietf-wson-topology.yang are
         link template" and ending with "Augment unreserved bandwidth of TE
         link template" in ietf-layer0-types are commented out.
      6. The descriptions at the module level could contain more information than
         just a one liner and the copyright.
      7. The "Security Considerations" section in the draft doesn't follow the 
         template in https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines
         and doesn't enummarate the corresponding leaves their delta exposures.

Nits:

      1. Normally revision description start with the "Initial Version". 
      2. Changes references to "Yang" to "YANG". 

Suggested Edits:

diff -c draft-ietf-ccamp-wson-yang-15.txt.orig draft-ietf-ccamp-wson-yang-15.txt
*** draft-ietf-ccamp-wson-yang-15.txt.orig                    2018-11-13 15:03:36.000000000 -0500
--- draft-ietf-ccamp-wson-yang-15.txt                         2018-11-13 16:03:35.000000000 -0500
***************
*** 17,23 ****
  
                                                         October 22, 2018
  
!                 A Yang Data Model for WSON Optical Networks

                         draft-ietf-ccamp-wson-yang-15
--- 17,23 ----
  
                                                         October 22, 2018
  
!                 A YANG Data Model for WSON Optical Networks

                         draft-ietf-ccamp-wson-yang-15
***************
*** 101,107 ****
     This document provides a YANG data model for the routing and
     wavelength assignment (RWA) Traffic Engineering (TE) topology in
     wavelength switched optical networks (WSONs). The YANG model
!    described in this document is a WSON technology-specific Yang model
     based on the information model developed in [RFC7446] and the two

--- 101,107 ----
     This document provides a YANG data model for the routing and
     wavelength assignment (RWA) Traffic Engineering (TE) topology in
     wavelength switched optical networks (WSONs). The YANG model
!    described in this document is a WSON technology-specific YANG model
     based on the information model developed in [RFC7446] and the two

***************
*** 1213,1219 ****
  
    description
      "This module contains a collection of YANG definitions for
!     RWA WSON.
  
      Copyright (c) 2018 IETF Trust and the persons identified
      as authors of the code.  All rights reserved.
--- 1213,1219 ----
  
    description
      "This module contains a collection of YANG definitions for
!      RWA WSON.
  
      Copyright (c) 2018 IETF Trust and the persons identified
      as authors of the code.  All rights reserved.
***************
*** 1237,1243 ****
  Internet-Draft             WSON YANG Model                 October 2018

!       "RFC XXX: A Yang Data Model for WSON Optical Networks ";
    }
  
    /*
--- 1237,1243 ----
  Internet-Draft             WSON YANG Model                 October 2018

!       "RFC XXX: A YANG Data Model for WSON Optical Networks ";
    }
  
    /*
***************
*** 1275,1281 ****
        }
        leaf payload-type {
          type string;
!         description "the payload type supported by this client tp";
          reference
            "http://www.iana.org/assignments/gmpls-sig-parameters
             /gmpls-sig-parameters.xhtml";
--- 1275,1281 ----
        }
        leaf payload-type {
          type string;
!         description "the payload type supported by this client TP";
          reference
            "http://www.iana.org/assignments/gmpls-sig-parameters
             /gmpls-sig-parameters.xhtml";
***************
*** 1292,1304 ****

        description
!         "Indicating if it is a client-facing TP.";
      }
    }
  
    grouping wson-ttp-attributes {
      description
!      "WSON tunnel termination point (e.g.tranponder)
         attributes";
  
      leaf-list supported-operational-modes {
--- 1292,1304 ----

        description
!         "Indicates if it is a client-facing TP.";
      }
    }
  
    grouping wson-ttp-attributes {
      description
!      "WSON tunnel termination point (e.g., tranponder)
         attributes";
  
      leaf-list supported-operational-modes {
***************
*** 1348,1355 ****
        type boolean;
        description
         "Indicates if the TTP, or transponder, is tunable. Tunable
!        transponders are assumed to be fully tunable to any of the
!        96 channels within DWDM C-band.";
      }
  
     leaf max-subcarrier-channel-num {
--- 1348,1355 ----
        type boolean;
        description
         "Indicates if the TTP, or transponder, is tunable. Tunable
!         transponders are assumed to be fully tunable to any of the
!         96 channels within DWDM C-band.";
      }
  
     leaf max-subcarrier-channel-num {
***************
*** 1359,1366 ****
       default 1;
       description
         "Indicate the maximum number of subcarrier channels for
!        super-channel transponders. When the value equals 1 it
!        represents regular single-channel transponder.";
     }
    }
  
--- 1359,1366 ----
       default 1;
       description
         "Indicate the maximum number of subcarrier channels for
!         super-channel transponders. When the value equals 1 it
!         represents regular single-channel transponder.";
     }
    }
  
***************
*** 1482,1488 ****
    }
  
    /* Augment bandwidth path constraints of connectivity-matrices information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:path-constraints/tet:te-bandwidth/tet:technology" {
--- 1482,1488 ----
    }
  
    /* Augment bandwidth path constraints of connectivity-matrices information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:path-constraints/tet:te-bandwidth/tet:technology" {
***************
*** 1497,1503 ****
    }
  
    /* Augment bandwidth path constraints of connectivity-matrix information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
  
--- 1497,1503 ----
    }
  
    /* Augment bandwidth path constraints of connectivity-matrix information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
  
***************
*** 2185,2191 ****
    }
  
    /* Augment label restrictions start of connectivity-matrices information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/"
          + "tet:connectivity-matrices/tet:label-restrictions/"
--- 2185,2191 ----
    }
  
    /* Augment label restrictions start of connectivity-matrices information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/"
          + "tet:connectivity-matrices/tet:label-restrictions/"
***************
*** 2209,2216 ****
  Internet-Draft             WSON YANG Model                 October 2018

!   /* Augment label restrictions end of connectivity-matrices information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/"
          + "tet:connectivity-matrices/tet:label-restrictions/"
--- 2209,2216 ----
  Internet-Draft             WSON YANG Model                 October 2018

!   /* Augment Restrictions end of connectivity-matrices information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/"
          + "tet:connectivity-matrices/tet:label-restrictions/"
***************
*** 2228,2234 ****
    }
  
    /* Augment label hop of underlay primary path of connectivity-matrices
! information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:underlay/tet:primary-path/tet:path-element/tet:type/"
--- 2228,2234 ----
    }
  
    /* Augment label hop of underlay primary path of connectivity-matrices
!      information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:underlay/tet:primary-path/tet:path-element/tet:type/"
***************
*** 2245,2251 ****
    }
  
    /* Augment label hop of underlay backup path of connectivity-matrices
! information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:underlay/tet:backup-path/tet:path-element/tet:type/"
--- 2245,2251 ----
    }
  
    /* Augment label hop of underlay backup path of connectivity-matrices
!      information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:underlay/tet:backup-path/tet:path-element/tet:type/"
***************
*** 2269,2275 ****
    }
  
    /* Augment label hop of route-exclude of connectivity-matrices information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:optimizations/tet:algorithm/tet:metric/"
--- 2269,2275 ----
    }
  
    /* Augment label hop of route-exclude of connectivity-matrices information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:optimizations/tet:algorithm/tet:metric/"
***************
*** 2289,2295 ****
    }
  
    /* Augment label hop of route-include of connectivity-matrices information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:optimizations/tet:algorithm/tet:metric/"
--- 2289,2295 ----
    }
  
    /* Augment label hop of route-include of connectivity-matrices information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:optimizations/tet:algorithm/tet:metric/"
***************
*** 2309,2315 ****
    }
  
    /* Augment label hop of path-route of connectivity-matrices information-
! source */

  Lee, et al.              Expires August 2018                  [Page 42]
--- 2309,2315 ----
    }
  
    /* Augment label hop of path-route of connectivity-matrices information-
!      source */

  Lee, et al.              Expires August 2018                  [Page 42]
***************
*** 2334,2340 ****
    }
  
    /* Augment ingress label restrictions of connectivity-matrix information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2334,2340 ----
    }
  
    /* Augment ingress label restrictions of connectivity-matrix information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2349,2355 ****
    }
  
    /* Augment ingress label restrictions start of connectivity-matrix
! information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2349,2355 ----
    }
  
    /* Augment ingress label restrictions start of connectivity-matrix
!      information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2375,2381 ****
    }
  
    /* Augment ingress label restrictions end of connectivity-matrix
! information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2375,2381 ----
    }
  
    /* Augment ingress label restrictions end of connectivity-matrix
!      information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2394,2400 ****
    }
  
    /* Augment egress label restrictions of connectivity-matrix information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2394,2400 ----
    }
  
    /* Augment egress label restrictions of connectivity-matrix information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2409,2415 ****
    }
  
    /* Augment egress label restrictions start of connectivity-matrix
! information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2409,2415 ----
    }
  
    /* Augment egress label restrictions start of connectivity-matrix
!      information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2434,2440 ****
    }
  
    /* Augment egress label restrictions end of connectivity-matrix information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2434,2440 ----
    }
  
    /* Augment egress label restrictions end of connectivity-matrix information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2452,2458 ****
    }
  
    /* Augment label hop of underlay primary path of connectivity-matrix
! information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2452,2458 ----
    }
  
    /* Augment label hop of underlay primary path of connectivity-matrix
!      information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2470,2476 ****
    }
  
    /* Augment label hop of underlay backup path of connectivity-matrix
! information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"

--- 2470,2476 ----
    }
  
    /* Augment label hop of underlay backup path of connectivity-matrix
!      information-source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"

***************
*** 2495,2501 ****
    }
  
    /* Augment label hop of route-exclude of connectivity-matrix information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2495,2501 ----
    }
  
    /* Augment label hop of route-exclude of connectivity-matrix information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2516,2522 ****
    }
  
    /* Augment label hop of route-include of connectivity-matrix information-
! source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2516,2522 ----
    }
  
    /* Augment label hop of route-include of connectivity-matrix information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2543,2550 ****
      }
    }
  
!   /* Augment label hop of path-route of connectivity-matrix information-source
! */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
--- 2543,2550 ----
      }
    }
  
!   /* Augment label hop of path-route of connectivity-matrix information-
!      source */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:information-source-entry/tet:connectivity-matrices/"
          + "tet:connectivity-matrix/"
***************
*** 2618,2625 ****
      }
    }
  
!   /* Augment label hop of underlay primary path of local-link-connectivities
! */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
--- 2618,2625 ----
      }
    }
  
!   /* Augment label hop of underlay primary path of local-link-
!      connectivities */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
***************
*** 2643,2649 ****
  
    }
  
!   /* Augment label hop of underlay backup path of local-link-connectivities */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
--- 2643,2650 ----
  
    }
  
!   /* Augment label hop of underlay backup path of local-link-
!      connectivities */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
***************
*** 2783,2789 ****
    }
  
    /* Augment label hop of underlay primary path of local-link-connectivity
! (LLC) */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
--- 2784,2790 ----
    }
  
    /* Augment label hop of underlay primary path of local-link-connectivity
!      (LLC) */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
***************
*** 2809,2815 ****
    }
  
    /* Augment label hop of underlay backup path of local-link-connectivity
! (LLC) */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
--- 2810,2816 ----
    }
  
    /* Augment label hop of underlay backup path of local-link-connectivity
!      (LLC) */
    augment "/nw:networks/nw:network/nw:node/tet:te/"
          + "tet:tunnel-termination-point/"
          + "tet:local-link-connectivities/"
***************
*** 3190,3196 ****
      }
      units THz;
      description
!       "The DWDM frequency in THz, e.g. 193.12500";
      reference
        "RFC6205";
    }
--- 3191,3197 ----
      }
      units THz;
      description
!       "The DWDM frequency in THz, e.g., 193.12500";
      reference
        "RFC6205";
    }
***************
*** 3201,3207 ****
      }
      units GHz;
      description
!       "The DWDM frequency in GHz, e.g. 193125.00";
      reference
        "RFC6205";
    }
--- 3202,3208 ----
      }
      units GHz;
      description
!       "The DWDM frequency in GHz, e.g., 193125.00";
      reference
        "RFC6205";
    }
***************
*** 3276,3282 ****
      description
        "This WA method selects the wavelength that
         has the largest residual capacity on the most loaded
!        link along the route (in muli-fiber networks).";
    }
  
    identity layer0-grid-type {
--- 3277,3283 ----
      description
        "This WA method selects the wavelength that
         has the largest residual capacity on the most loaded
!        link along the route (in multi-fiber networks).";
    }
  
    identity layer0-grid-type {
***************
*** 3323,3335 ****
    identity bw-otu1e {
      base layer0-bandwidth-type;
      description
!       "OTU1e(11.04G)";
    }
  
    identity bw-otu1f {
      base layer0-bandwidth-type;
      description
!       "OTU1f(11.27G)";
    }
  
    identity bw-otu2 {
--- 3324,3336 ----
    identity bw-otu1e {
      base layer0-bandwidth-type;
      description
!       "OTU1e (11.04G)";
    }
  
    identity bw-otu1f {
      base layer0-bandwidth-type;
      description
!       "OTU1f (11.27G)";
    }
  
    identity bw-otu2 {
***************
*** 3538,3544 ****
          type frequency-thz;
            description
              "The DWDM fixed-grid channel frequency in THz,
!           e.g. 193.12500";
            reference
              "RFC6205";
          }
--- 3539,3545 ----
          type frequency-thz;
            description
              "The DWDM fixed-grid channel frequency in THz,
!              e.g., 193.12500";
            reference
              "RFC6205";
          }
***************
*** 3555,3561 ****
            type uint32;
            units nm;
            description
!             "The CWDM wavelength in nanometer, e.g. 1511";
            reference
              "RFC6205";
          }
--- 3556,3562 ----
            type uint32;
            units nm;
            description
!             "The CWDM wavelength in nanometer, e.g., 1511";
            reference
              "RFC6205";
          }
***************
*** 3573,3591 ****
         choice single-or-super-channel {
          description "single of super channel";
          case single {
!             leaf channel-freq {
!               type frequency-thz;
             description
               "The DWDM fixed-grid channel frequency in THz,
!               e.g. 193.12500";
!             }
          }
          case super {
            leaf-list subcarrier-channels {
!               type frequency-thz;
!               description
!                 "List of subcarrier channels for super channel.";
!             }
          }
        }
  
--- 3574,3592 ----
         choice single-or-super-channel {
          description "single of super channel";
          case single {
!           leaf channel-freq {
!             type frequency-thz;
             description
               "The DWDM fixed-grid channel frequency in THz,
!               e.g., 193.12500";
!           }
          }
          case super {
            leaf-list subcarrier-channels {
!             type frequency-thz;
!             description
!               "List of subcarrier channels for super channel.";
!            }
          }
        }
  
***************
*** 3602,3608 ****
  Internet-Draft             WSON YANG Model                 October 2018

!             "The CWDM wavelength in nanometer, e.g. 1511";
            reference
              "RFC6205";
          }
--- 3603,3609 ----
  Internet-Draft             WSON YANG Model                 October 2018

!             "The CWDM wavelength in nanometer, e.g., 1511";
            reference
              "RFC6205";
          }
***************
*** 3630,3636 ****
      description "Label step information for WSON";
      choice layer0-grid-type {
        description
!         " WSON grid-type: DWDM, CWDM, etc.";
        case dwdm {
          leaf wson-dwdm {
            type identityref {
--- 3631,3637 ----
      description "Label step information for WSON";
      choice layer0-grid-type {
        description
!         "WSON grid-type: DWDM, CWDM, etc.";
        case dwdm {
          leaf wson-dwdm {
            type identityref {
***************
*** 3712,3718 ****
        type frequency-thz;
        description
          "The DWDM flex-grid channel central frequency
!          in THz, e.g. 193.12500.";
        reference
          "RFC7698";
      }
--- 3713,3719 ----
        type frequency-thz;
        description
          "The DWDM flex-grid channel central frequency
!          in THz, e.g., 193.12500.";
        reference
          "RFC7698";
      }
***************
*** 3723,3729 ****
      leaf central-frequency {
        type frequency-thz;
        description
!         "Flex-grid central frequency in THz, e.g. 193.12500.";
        reference
          "RFC7698";
      }
--- 3724,3730 ----
      leaf central-frequency {
        type frequency-thz;
        description
!         "Flex-grid central frequency in THz, e.g., 193.12500.";
        reference
          "RFC7698";
      }
***************
*** 3731,3737 ****
      leaf slot-width {
        type frequency-ghz;
        description
!         "Flex-grid The DWDM slot width in GHz, e.g. 50, 100, 150.";
        reference
          "RFC7698";
      }
--- 3732,3738 ----
      leaf slot-width {
        type frequency-ghz;
        description
!         "Flex-grid The DWDM slot width in GHz, e.g.., 50, 100, 150.";
        reference
          "RFC7698";
      }
***************
*** 3748,3756 ****
           list subcarrier-channels {
             key central-frequency;
             uses flex-grid-channel;
!          description
                "List of subcarrier channels for flex-grid
!              super channel.";

  Lee, et al.              Expires August 2018                  [Page 69]
--- 3749,3757 ----
           list subcarrier-channels {
             key central-frequency;
             uses flex-grid-channel;
!            description
                "List of subcarrier channels for flex-grid
!                super channel.";

  Lee, et al.              Expires August 2018                  [Page 69]
***************
*** 3767,3773 ****
    grouping flex-grid-label-restriction {
      description
       "Flex Grid-specific label restriction.";
!    uses layer0-label-restriction;
  
      container flex-grid {
       description "flex-grid definition";
--- 3768,3774 ----
    grouping flex-grid-label-restriction {
      description
       "Flex Grid-specific label restriction.";
!     uses layer0-label-restriction;
  
      container flex-grid {
       description "flex-grid definition";

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 14 Nov 2018 09:28:05 +0100 (CET)

Hi,

To YANG doctors in general, and the authors of this draft.

>       4. The indentation in ietf-layer0-types is not consistent. I

The RFC editor nowadays tries to enforce consistent indentation and
style across all YANG modules published in RFCs.  They ask us to help
with this by informing authors.  The RFC editor can run tools and
manually fix indentation, but since they don't really know YANG, they
prefer that the changes they make are minimal.

The recommendation to authors is to run

  $ pyang --ietf --max-line-length 69 <file>

when this is ok run:

  $ pyang -f yang --keep-comments <file> > <outfile>

Unfortunately, the tool isn't 100% perfect, in that it won't ensure
that all lines are short enough.  Specifically, the argument to "path"
gets printed on a single line.

So the recommendation is to run this tool, and then manually tweak the
long statements, e.g. run

   $ diff <oldfile> <newfile>

to find the diffs.

/martin

---

From: Ladislav Lhotka <[email-masked]>
Date: Wed, 14 Nov 2018 09:55:25 +0100

>   $ pyang -f yang --keep-comments <file> > <outfile>

Another option is to run

    $ pyang -f yin -o ymod.yin --keep-comments ymod-orig.yang

and then

    $ xsltproc -o ymod.yang yin2yang.xsl ymod.yin

The yin2yang.xsl stylesheet is available here:

https://github.com/llhotka/YANG-I-D/tree/master/.tools/xslt

In my experience, the stylesheet also does a decent job in breaking long lines.
By default, it tries to break at a space character, another character may be
suggested by using the "delim" processing instruction.

For example,

    <pattern value="..."><?delim )?></pattern>

instructs the stylesheet to break the argument after a right parenthesis.

Lada

> _______________________________________________
> yang-doctors mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/yang-doctors
-- 
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Wed, 14 Nov 2018 12:35:56 +0000

Martin, Lada,

Thanks - this is useful. Many times I've had to fix models lines during xml2rfc iterations. In fact, I wrote a crude python script to print lines in a file exceeding an input length. 

Thanks,
Acee 

On 11/14/18, 3:55 AM, "Ladislav Lhotka" <[email-masked]> wrote:

    Another option is to run

    and then

    

    For example,

    
    Lada

---

From: Martin Bjorklund <[email-masked]>
Date: Wed, 14 Nov 2018 13:44:06 +0100 (CET)

"Acee Lindem (acee)" <[email-masked]> wrote:
> Martin, Lada,
> 
> Thanks - this is useful. Many times I've had to fix models lines
> during xml2rfc iterations. In fact, I wrote a crude python script to
> print lines in a file exceeding an input length.

Also, the tool is getting better from time to time; I have a PR that I
haven't looked at yet which supposedly allows pyang -f yang
--max-line-length 69, so that long arguments will be pretty-printed
across several lines.

/martin

---

From: Ladislav Lhotka <[email-masked]>
Date: Wed, 14 Nov 2018 13:48:54 +0100

On Wed, 2018-11-14 at 12:35 +0000, Acee Lindem (acee) wrote:
> Martin, Lada,
> 
> Thanks - this is useful. Many times I've had to fix models lines during
> xml2rfc iterations. In fact, I wrote a crude python script to print lines in a
> file exceeding an input length.

Right - that's why I prefer writing both draft text and YANG modules as XML.

Lada

---

From: Leeyoung <[email-masked]>
Date: Wed, 14 Nov 2018 19:33:21 +0000

Hi Acee,

Thanks for your YANG Doctor's review on draft-ietf-ccamp-wson-yang-15. 

You said:     
    > >       4. The indentation in ietf-layer0-types is not consistent. I
    > >       tried to fix

I fixed all indentation issues and excessive lines. Here's the updated file attached. Are there any issues with the draft and YANG issues that need to be fixed? 

Thanks.
Young
-----Original Message-----
From: Acee Lindem (acee) [mailto:[email-masked]] 
Sent: Wednesday, November 14, 2018 6:36 AM
To: Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15

Martin, Lada,

Thanks,
Acee 

    
    Another option is to run

    and then

    

    For example,

    
    Lada

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Wed, 14 Nov 2018 19:43:56 +0000

Hi Young, 

This was merely a follow-up Email on a single comment with the other pruned. Did you miss the Email with the complete review (attached)? 

Thanks,
Aceee

On 11/14/18, 2:33 PM, "Leeyoung" <[email-masked]> wrote:

    Hi Acee,

    
    
    Thanks.
    Young
    -----Original Message-----
    From: Acee Lindem (acee) [mailto:[email-masked]] 
    Sent: Wednesday, November 14, 2018 6:36 AM
    To: Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
    Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
    
    Martin, Lada,

    Thanks,
    Acee 

        On Wed, 2018-11-14 at 09:28 +0100, Martin Bjorklund wrote:
        > >       tried to fix.
        
        Another option is to run

        and then

        

        For example,

        
        Lada

    

Major Issues: None

Minor Issues: 

Nits:

Suggested Edits:

                                                         October 22, 2018

  
                                                         October 22, 2018

--- 101,107 ----

***************
*** 1213,1219 ****
  
    description

    description

  
    /*
--- 1237,1243 ----

    /*
***************
*** 1275,1281 ****
        }
        leaf payload-type {
          type string;

        description

        description

  

--- 1359,1366 ----
       default 1;
       description
  
***************
*** 1482,1488 ****
    }

  
  
--- 1497,1503 ----
    }

***************
*** 2185,2191 ****
    }

  

  

  

  

  

  

  

  

  

--- 2470,2476 ----
    }

***************
*** 2495,2501 ****
    }

  

  

  
    }

    }

  

  

    identity bw-otu1f {
  
    identity bw-otu2 {
--- 3324,3336 ----
    identity bw-otu1e {
  
    identity bw-otu1f {
  
    identity bw-otu2 {
***************
*** 3538,3544 ****
          type frequency-thz;
            description
  
--- 3574,3592 ----
  
***************
*** 3602,3608 ****

  
      container flex-grid {
  
      container flex-grid {

---

From: Leeyoung <[email-masked]>
Date: Wed, 14 Nov 2018 19:47:26 +0000

Hi Acee,

I must have missed the previous email. Thanks for the attachment and your timely and extensive review. I will follow through your comments. 

Best regards,
Young

-----Original Message-----
From: Acee Lindem (acee) [mailto:[email-masked]] 
Sent: Wednesday, November 14, 2018 1:44 PM
To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15

Hi Young, 

Thanks,
Aceee

    Hi Acee,

    
    
    Thanks.
    Young
    -----Original Message-----
    From: Acee Lindem (acee) [mailto:[email-masked]] 
    Sent: Wednesday, November 14, 2018 6:36 AM
    To: Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
    Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
    
    Martin, Lada,

    Thanks,
    Acee 

        
        Another option is to run

        and then

        

        For example,

        
        Lada

---

From: Leeyoung <[email-masked]>
Date: Fri, 16 Nov 2018 01:09:56 +0000

Hi Acee,

Please see the diff file between version 15 and version 16. I just wanted to check with you if the revision (v16) satisfies you. I believe I did address all of your comments faithfully and corrected all the nits. But in case there are still issues to resolve, please let me know. Thanks.

Best regards,
Young
-----------------------
The followings are my comments to your comments raised during your Yang doctor review. 

Minor Issues: 

      1. The document has 7 authors. This will have to be ok'ed by the 
         responsible AD. 
YL>> now 5 authors in the front page.

YL>> removed periods in all YANG descriptions. 

YL>> More words in the type descriptions are supplied; expanded some key terms. 

YL>> Fixed. 

YL>> Commented-out portions are removed. These augmentation statements are augmenting the topology templates for which there is no network-types attributes. In other words, if you follow the path in the when statement you will not go to an existing leaf in the YANG tree. 

YL>> All exterior comments are moved to the augmentation descriptions. 

      7. When conditions starting with "Augment label hop of underlay primary path

YL>> See my comment for #5.
 
      8. The descriptions at the module level could contain more information than
         just a one liner and the copyright.

YL>> Added more detailed text. 

YL>> Supplied. 

Nits:

YL>> Corrected. 

YL>> Corrected. 

In addition, all nits have been fixed. 

-----Original Message-----
From: Acee Lindem (acee) [mailto:[email-masked]] 
Sent: Wednesday, November 14, 2018 1:44 PM
To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15

Hi Young, 

Thanks,
Aceee

    Hi Acee,

    
    
    Thanks.
    Young
    -----Original Message-----
    From: Acee Lindem (acee) [mailto:[email-masked]] 
    Sent: Wednesday, November 14, 2018 6:36 AM
    To: Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
    Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
    
    Martin, Lada,

    Thanks,
    Acee 

        
        Another option is to run

        and then

        

        For example,

        
        Lada

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Sat, 17 Nov 2018 17:44:41 +0000

Hi Young, 

The changes look good. The one question I have is whether you should expand the optical acronyms on first use in the ietf-layer0-types.yang. Even if they are defined in other documents, the models need to stand on their own. I guess the question is whether anyone using types in the first place would know all these acronyms. I'm talking about TP, TTP, PID, OADW, ROADM, CWDM, DWDM, FEC, G-FEC, E-FEC, ODU, OPU, OTU, OXC, and potentially others. I'll leave it up to you. 

Thanks,
Acee

On 11/15/18, 8:22 PM, "Leeyoung" <[email-masked]> wrote:

    Hi Acee,

    Best regards,
    Young
    -----------------------
    
    Minor Issues: 

    

    
    
    YL>> Fixed. 

    

    

    
     
    YL>> Supplied. 
    
    Nits:

    YL>> Corrected. 

    YL>> Corrected. 

    
    -----Original Message-----
    From: Acee Lindem (acee) [mailto:[email-masked]] 
    Sent: Wednesday, November 14, 2018 1:44 PM
    To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
    Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
    
    Hi Young, 

    Thanks,
    Aceee

        Hi Acee,

        
        
        Thanks.
        Young
        -----Original Message-----
        From: Acee Lindem (acee) [mailto:[email-masked]] 
        Sent: Wednesday, November 14, 2018 6:36 AM
        To: Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
        Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
        
        Martin, Lada,

        Thanks,
        Acee 

            
            Another option is to run

            and then

            

            For example,

            
            Lada

---

From: Leeyoung <[email-masked]>
Date: Mon, 19 Nov 2018 17:56:53 +0000

Hi Acee,

Thanks for your quick reply and pointing out specific acronyms to be expanded. I agree with you on the expansion of the acronyms, which is incorporated in the proposed revision (v16). Please see the attached diff file that compares v15 and v16. I will upload version 16 shortly. Please let me know if there are still issues to be corrected. 

Thanks & Best regards,
Young

-----Original Message-----
From: Acee Lindem (acee) [mailto:[email-masked]] 
Sent: Saturday, November 17, 2018 11:45 AM
To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15

Hi Young, 

Thanks,
Acee

    Hi Acee,

    Best regards,
    Young
    -----------------------
    
    Minor Issues: 

    

    
    
    YL>> Fixed. 

    

    

    
     
    YL>> Supplied. 
    
    Nits:

    YL>> Corrected. 

    YL>> Corrected. 

    
    -----Original Message-----
    From: Acee Lindem (acee) [mailto:[email-masked]] 
    Sent: Wednesday, November 14, 2018 1:44 PM
    To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
    Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
    
    Hi Young, 

    Thanks,
    Aceee

        Hi Acee,

        
        
        Thanks.
        Young
        -----Original Message-----
        From: Acee Lindem (acee) [mailto:[email-masked]] 
        Sent: Wednesday, November 14, 2018 6:36 AM
        To: Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
        Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
        
        Martin, Lada,

        Thanks,
        Acee 

            
            Another option is to run

            and then

            

            For example,

            
            Lada

---

From: "Acee Lindem (acee)" <[email-masked]>
Date: Tue, 20 Nov 2018 11:55:02 +0000

Hi Young, 

Thanks for the update. This version looks good. I had actually misinterpreted FEC as MPLS Forwarding Equivalence Class rather than Forward Error Correction ;^) 

Thanks,
Acee

On 11/19/18, 1:07 PM, "Leeyoung" <[email-masked]> wrote:

    Hi Acee,

    Thanks & Best regards,
    Young
    
    -----Original Message-----
    From: Acee Lindem (acee) [mailto:[email-masked]] 
    Sent: Saturday, November 17, 2018 11:45 AM
    To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
    Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
    
    Hi Young, 

    Thanks,
    Acee

        Hi Acee,

        Best regards,
        Young
        -----------------------
        
        Minor Issues: 

        

        
        
        YL>> Fixed. 

        

        

        
         
        YL>> Supplied. 
        
        Nits:

        YL>> Corrected. 

        YL>> Corrected. 

        
        -----Original Message-----
        From: Acee Lindem (acee) [mailto:[email-masked]] 
        Sent: Wednesday, November 14, 2018 1:44 PM
        To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
        Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
        
        Hi Young, 

        Thanks,
        Aceee

            Hi Acee,

            
            
            Thanks.
            Young
            -----Original Message-----
            
            Martin, Lada,

            Thanks,
            Acee 

                
                Another option is to run

                and then

                

                For example,

                
                Lada

---

From: Leeyoung <[email-masked]>
Date: Tue, 20 Nov 2018 14:55:05 +0000

Hi Acee,

Great! I just uploaded the revision (v16). Thanks a lot for your thorough comments and a quick response. 

Thanks,
Young

-----Original Message-----
From: Acee Lindem (acee) [mailto:[email-masked]] 
Sent: Tuesday, November 20, 2018 5:55 AM
To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]; [email-masked]
Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15

Hi Young, 

Thanks,
Acee

    Hi Acee,

    Thanks & Best regards,
    Young
    
    -----Original Message-----
    From: Acee Lindem (acee) [mailto:[email-masked]] 
    Sent: Saturday, November 17, 2018 11:45 AM
    To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
    Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
    
    Hi Young, 

    Thanks,
    Acee

        Hi Acee,

        Best regards,
        Young
        -----------------------
        
        Minor Issues: 

        

        
        
        YL>> Fixed. 

        

        

        
         
        YL>> Supplied. 
        
        Nits:

        YL>> Corrected. 

        YL>> Corrected. 

        
        -----Original Message-----
        From: Acee Lindem (acee) [mailto:[email-masked]] 
        Sent: Wednesday, November 14, 2018 1:44 PM
        To: Leeyoung <[email-masked]>; Ladislav Lhotka <[email-masked]>; Martin Bjorklund <[email-masked]>; [email-masked]; [email-masked]
        Subject: Re: [yang-doctors] [CCAMP] Yangdoctors last call review of draft-ietf-ccamp-wson-yang-15
        
        Hi Young, 

        Thanks,
        Aceee

            Hi Acee,

            
            
            Thanks.
            Young
            -----Original Message-----
            
            Martin, Lada,

            Thanks,
            Acee 

                
                Another option is to run

                and then

                

                For example,

                
                Lada