Subject: [babel] Éric Vyncke's Discuss on draft-ietf-babel-yang-model-10: (with DISCUSS and COMMENT)

From: Éric Vyncke via Datatracker <[email-masked]>
Date: Mon, 17 May 2021 02:45:59 -0700

Éric Vyncke has entered the following ballot position for
draft-ietf-babel-yang-model-10: Discuss

When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)

Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about DISCUSS and COMMENT positions.

The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-babel-yang-model/

----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------

Thank you for the work put into this document. I really like the fact that the
information model is built before the data model. Congratulations !

Please find below one blocking (but trivial to fix) DISCUSS point, some
non-blocking COMMENT points (but replies would be appreciated), and one nit.

Thank you to Donald Eastlake for his shepherd's write-up (including the WG
consensus).

I hope that this helps to improve the document,

Regards,

-éric

== DISCUSS ==

The YANG module does not compile correctly with PYANG, it should be easy to fix
though :-) See: https://yangcatalog.org/results/[email-masked]
Or is it a PYANG error ?

----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------

== COMMENTS ==

The related links on should be updated. E.g., the YANG catalog entry should be:
https://www.yangcatalog.org/yang-search/module_details.php?module=ietf-babel@2021-05-12

-- Section 2.2 --
I usually use the expanded tree view rather the YANG module itself to get a
global view. Is there any reason why the full tree view is not included?

-- Section 5 --
Is there any reason why the doc shepherd is not acknowledged ?

== NITS ==

-- Section 2.3 --
s/MAC based security/MAC-based security/ ?

---

From: Donald Eastlake <[email-masked]>
Date: Tue, 18 May 2021 22:29:24 -0400

Hi Éric,

On Mon, May 17, 2021 at 5:46 AM Éric Vyncke via Datatracker
<[email-masked]> wrote:
> ...
> ...

You're welcome.

> ...
> ...
> ...

I don't feel any need to be listed in the acknowledgements. While I
did do the Shepherd write up (PROTO), I really don't recall making any
suggestions for change to this draft which is by two experienced
authors. My general policy, when I am an author, is to acknowledge
people whose comments resulted in technical or substantial editorial
changes, If the Shepherd should be listed just because they are the
Shepherd then why shouldn't the Chair(s) and AD be listed in the
Acknowledgements?

Thanks,
Donald
===============================
 Donald E. Eastlake 3rd   +1-508-333-2270 (cell)
 2386 Panoramic Circle, Apopka, FL 32703 USA
 [email-masked]

---

From: Mahesh Jethanandani <[email-masked]>
Date: Tue, 18 May 2021 21:10:17 -0700

Hi Eric,

Thank you for the review. See inline.

> On May 17, 2021, at 2:45 AM, Éric Vyncke via Datatracker <[email-masked]> wrote:
> 

This is a tooling error (even though I have gone ahead and changed it in the model). The ABNF grammar says the following for identity-stmt in RFC 7950.

   identity-stmt       = identity-keyword sep identifier-arg-str optsep
                         (";" /
                          "{" stmtsep
                              ;; these stmts can appear in any order
                              *if-feature-stmt
                              *base-stmt
                              [status-stmt]
                              [description-stmt]
                              [reference-stmt]
                          "}") stmtsep

According to it, the ‘if-feature’ and ‘base-stmt’ can appear in any order. But for some reason pyang version 2.4.0 insists on a particular order. I will bring this up with tooling folks.

Added this to the Appendix.

> s/MAC based security/MAC-based security/ ?

Fixed.

Thanks.

Mahesh Jethanandani
[email-masked]

---

From: Mahesh Jethanandani <[email-masked]>
Date: Wed, 19 May 2021 08:18:16 -0700

Hi Martin,

> On May 18, 2021, at 11:40 PM, Martin Björklund <[email-masked]> wrote:
> 
> Hi,
> 
>>> 
>>> though :-) See:
>>> https://yangcatalog.org/results/[email-masked]
>>> Or is it a PYANG error ?
>> 
>> This is a tooling error (even though I have gone ahead and changed it
>> in the model). The ABNF grammar says the following for identity-stmt
>> in RFC 7950.
>> 
>> According to it, the ‘if-feature’ and ‘base-stmt’ can appear in any
>> order. But for some reason pyang version 2.4.0 insists on a particular
>> order. I will bring this up with tooling folks.
> 
> This is not a tooling error.  The error message is:
> 
>  [email-masked] <mailto:[email-masked]>:148: error: keyword "if-feature" not in
>    canonical order (see RFC 6020, Section 12)
> 
> [side note: the RFC ref in this error message should be updated to RFC
> 7950 Section 14]
> 
> The referenced section says:
> 
>   The ABNF grammar [RFC5234] [RFC7405] defines the canonical order.
> 
> When we publish YANG models in RFCs we use the canonical order in
> order to have a consistent format.  pyang checks this when the flag
> --ietf is used.

I am trying to understand, and this is more for my education, what rule within the grammar is determining the canonical order?

Also, should pyang report this as an error, or maybe a warning might be better? After all, this is being done for consistency reasons.

> /martin

Mahesh Jethanandani
[email-masked]