Re: [lp-wan] AD review of draft-ietf-lpwan-schc-yang-data-model-12

["Eric Vyncke (evyncke)" <[e-mail]>]

Laurent,

Reviewing your -13, it seems that the ‘all1’ was kept unchanged in section 3.10.3. But thanks for using “all-1” in the rest of the module. It is probably an oversight, please quickly fix it before midnight (cut-off date).

Nits: still a “maximum-packet-size rexpresses” in section 3.10.16 (this section is also rather dry and lacks an introduction sentence).

More important, the YANG module has changed, so there is need for a new revision date.

I also like the way you remove some text to avoid duplication with the YANG module itself.

All in all, good job !

Regards

-éric


From: lp-wan <[e-mail]> on behalf of Laurent Toutain <[e-mail]>
Date: Monday, 30 May 2022 at 19:33
To: "Eric Vyncke (evyncke)" <evyncke=[e-mail]>
Cc: "[e-mail]" <[e-mail]>
Subject: Re: [lp-wan] AD review of draft-ietf-lpwan-schc-yang-data-model-12

Thank you very much Eric for your comments.

I haven't issued a -13 since some comments such as the security section requires more work, but I pushed the editorial corrections in the repo.

There is two comments I would like to discuss in the list, before editing.

The first one is about the conversion between string and binary. That's a very good point. Currently it happens only for string fields such ad Uri-path and Uri-query in CoAP, RFC 7252 gives this definition of strings:


 string:   A Unicode string that is encoded using UTF-8 [RFC3629<https://datatracker.ietf.org/doc/html/rfc3629>] in

             Net-Unicode form [RFC5198<https://datatracker.ietf.org/doc/html/rfc5198>].

we can keep the same definition and say that the Target Value is the binary representation of this. But we may image other protocols that uses another coding, is it out of the scope of the YANG data model or we try to have a phrasing to include them ?


The second point is also raised by Tom in his review. "all1" term appears ambiguous because l and 1 are confusing. May be we can write them in capital in the model ALL1 ALL0 to limit the ambiguities ?

Laurent

On Mon, May 30, 2022 at 10:09 AM Eric Vyncke (evyncke) <evyncke=[e-mail]<mailto:[e-mail]>> wrote:
Dear Ana and Laurent,

Please find below a list of points raised during my AD review. I would appreciate a reply on all of them; the reply could vary from “we ignore because of foo” to “this will be fixed in -13”.

I have also attached your I-D formatted as a MS-Word document to allow you to chase all typos and grammar errors.

Thanks again for your work, YANG models are never a fun task to do.

Regards

-éric

# Sect #

no need for capitalized “Destination address”

# Sect 2

 *blocking* please use the right template from BCP14

#Sect 3

no need to repeat the introduction text from sect 1

# Section 3.2

no need to capitalize “Data Model”

please expand “RCS” at first use

# Sect 3.3

use either “YANG data model” or “YANG module” but not “YANG model”

s/always derives/is always derived/ ? (Passive voice)

s/MUST derive/MUST be derived/ ?

Suggest renaming the section into “Conventions for Field Identifiers”

# Sect 3.4

Please do not capitalize “byte” (and you may want to use the plural form)

# Sect 3.7

Please expand “CDA”

Unsure whether “strings must be converted to binary” is non-ambiguous. How can it be done ? Should there be a reference ?

# Sec 3.9

s/ shows some CDA definition/ shows some CDA definitions/

# Sec 3.10.2

It should be clear for the reader that this text is coming out of RFC 8724 and not specified by this I-D.

# Sec 3.10.3

Suggest to add somewhere that "all1" is lower case "ALL" followed by figure "1".

# Sec 3.10.4

It should be clear for the reader that this text is coming out of RFC 8724 and not specified by this I-D.

Why "All1" (capitalized) while previously "all1" (lowercase) was used ?

# Sec 3.10.5

"rexpresses" ?

# Sec 3.10.6

Should the text be clear that l2-word-size is expressed in bits ?

# Sec 4.2

s/milli-seconds for real time/milliseconds for real-time/

s/micro-second/microsecond/

s/ computed through/ computed by/ ?

An RFC must be accurate, so text like "of about 1.05 second" is not suitable. If it is about 2**20 microseconds, then be explicit.

# Section 5

Thanks for including this section. But, per RFC 7942 section 2.1, a note to the RFC editor must be included to remove this section before publication.

# Section 6

I am afraid that there must be some IANA considerations for each and every YANG module, e.g., see https://datatracker.ietf.org/doc/html/draft-ietf-dhc-dhcpv6-yang-25#section-6

# Section 7

Please use the YANG security template: https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines

# Section 9

The location of the YANG module is unusual, it should come before the "implementation status".

s/code begins/CODE BEGINS/ same for ends of course __

Please put "" around the filename.


_______________________________________________
lp-wan mailing list
[e-mail]<mailto:[e-mail]>
https://www.ietf.org/mailman/listinfo/lp-wan