Early Review of draft-ietf-opsawg-teas-common-ac-02
review-ietf-opsawg-teas-common-ac-02-yangdoctors-early-aries-2024-01-10-00

Request Review of draft-ietf-opsawg-teas-common-ac
Requested revision No specific revision (document currently at 14)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2024-01-12
Requested 2023-12-14
Requested by Tianran Zhou
Authors Mohamed Boucadair , Richard Roberts , Oscar Gonzalez de Dios , Samier Barguil , Bo Wu
I-D last updated 2024-01-10
Completed reviews Rtgdir Early review of -05 by Andy Smith (diff)
Yangdoctors Early review of -02 by Ebben Aries (diff)
Genart Last Call review of -13 by Behcet Sarikaya (diff)
Secdir Last Call review of -13 by Watson Ladd (diff)
Comments 
There is a group of related documents, including:
draft-ietf-opsawg-teas-common-ac
draft-ietf-opsawg-teas-attachment-circuit
draft-ietf-opsawg-ntw-attachment-circuit
draft-ietf-opsawg-ac-lxsm-lxnm-glue
Assignment Reviewer Ebben Aries
State Completed
Request Early review on draft-ietf-opsawg-teas-common-ac by YANG Doctors Assigned
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/KJIQEXvmLR2shfGLXkgdRGt5-Sc
Reviewed revision 02 (document currently at 14)
Result On the right track
Completed 2024-01-10
review-ietf-opsawg-teas-common-ac-02-yangdoctors-early-aries-2024-01-10-00
1 module in this draft:
- ietf-ac-common@2023-11-13.yang

YANG compiler errors or warnings (pyang 2.6.0, yanglint 2.1.128, yangson 1.4.19)
- No compiler errors or warnings

NOTE: This module was reviewed and validated (stub instance-data) in
conjunction with draft-ietf-opsawg-teas-attachment-circuit-03 and I did my
best to separate comments out to each even though validation crosses the 2
reviews

General comments on the draft:
- Section 3: There is reference to the tree and the tree being too long but
  this module is solely groupings, identities and typedefs thus there is no
  implementation of a standalone data tree in this common module.  Is this
  moreso in reference to a tree output that might include
  `--tree-print-groupings` as seen in a subsequent section?
- Section 4: Move the "file" declaration in <CODE BEGINS> up to align and
  quote "ietf-ac-common@2023-11-13.yang" otherwise published IETF tooling will
  fail to parse correctly

General comments on the module:
- Suggestion: Insert appropriate line breaks throughout module statements for
  readability
- L#509: must statement needs to qualify identities as 'ac-common:slaac' and
  'ac-common:provider-dhcp-slaac' otherwise the imports/uses will assume
  localization (Suggest auditing all 'when' and 'must' statements that
  reference identities to ensure full qualification for any future imports)
- L#1329: Address/remove comment
- L#1404: s/type\{/type \{/
- Minor nit: s/Indciates/Indicates/ -> "Indciates the actual date and time
  when the service"
- For "vlan-id" related leaves, should these be scoped in the uint16 space?
- For the `pseudowire` and `vpls` groupings, there is a duplication of nodes.
  Does it make sense to consolidate as much as possible and only diverge where
  necessary?