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

[[e-mail]]

Hi Ebben, 

Thank you for the review. 

Updated the spec to take into account your comments as you can see at: https://author-tools.ietf.org/api/iddiff?doc_1=draft-ietf-opsawg-teas-common-ac&url_2=https://boucadair.github.io/attachment-circuit-model/draft-ietf-opsawg-teas-common-ac.txt

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.