Re: [yang-doctors] Yangdoctors early review of draft-ietf-opsawg-teas-common-ac-02

[Ebben Aries <[e-mail]>]

Thx Med for addressing my comments.  The updates look good to me.

/ebben

On 2024-01-11 14:30:09, [e-mail] wrote:
> [External Email. Be cautious of content]
> 
> 
> Hi Ebben,
> 
> Thank you for the review.
> 
> Updated the spec to take into account your comments as you can see at: https://urldefense.com/v3/__https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-opsawg-teas-common-ac&url_2=https:**Aboucadair.github.io*attachment-circuit-model*draft-ietf-opsawg-teas-common-ac.txt__;Ly8vLw!!NEt6yMaO-gk!DFSr2_ATQcCtt5PSDds7C4uS-HfuciT-F6O_lx3_dBJOFrX_WcRuzQ9NokZvhNBpfw0WB1Xwbh2xNcK3YEwuwywW$
> 
> Please see inline for more context.
> 
> Cheers,
> Med
> 
> > -----Message d'origine-----
> > De : Ebben Aries via Datatracker <[e-mail]>
> > Envoyé : mercredi 10 janvier 2024 23:57
> > À : [e-mail]
> > Cc : [e-mail];
> > [e-mail]
> > Objet : Yangdoctors early review of draft-ietf-opsawg-teas-
> > common-ac-02
> >
> > Reviewer: Ebben Aries
> > Review result: On the Right Track
> >
> > 1 module in this draft:
> > - [e-mail]
> >
> > 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
> >
> 
> [Med] Thank you.
> 
> > 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?
> 
> [Med] Yes because we were assuming that the reader is familiar with rfc8340#section-3.2. Added a note to make this explicit.
> 
> > - Section 4: Move the "file" declaration in <CODE BEGINS> up to
> > align and
> >   quote "[e-mail]" otherwise published IETF
> > tooling will
> >   fail to parse correctly
> 
> [Med] Fixed.
> 
> >
> > General comments on the module:
> > - Suggestion: Insert appropriate line breaks throughout module
> > statements for
> >   readability
> 
> [Med] I think that we are already following the practices for published modules in that matter: no line breaks inside the groupings/containers.
> 
> > - 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)
> 
> [Med] Fixed.
> 
> > - L#1329: Address/remove comment
> 
> [Med] Fixed.
> 
> > - L#1404: s/type\{/type \{/
> 
> [Med] Fixed.
> 
> > - Minor nit: s/Indciates/Indicates/ -> "Indciates the actual date
> > and time
> >   when the service"
> 
> [Med] Fixed.
> 
> > - For "vlan-id" related leaves, should these be scoped in the
> > uint16 space?
> 
> [Med] Yes, they should. Thanks for catching this. Fixed.
> 
> > - 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?
> >
> 
> [Med] I don't think there is value to do that here as we only have one common leaf. Thanks.
> 
> ____________________________________________________________________________________________________________
> Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
> a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
> Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.
> 
> This message and its attachments may contain confidential or privileged information that may be protected by law;
> they should not be distributed, used or copied without authorisation.
> If you have received this email in error, please notify the sender and delete this message and its attachments.
> As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
> Thank you.