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

[Laurent Toutain <[e-mail]>]

Hi Eric,

Thanks for the comment, I'm always afraid to push a YANG document in hurry
since they can create a lot of side effects.

On Mon, Jul 11, 2022 at 5:21 PM Eric Vyncke (evyncke) <[e-mail]>
wrote:

> 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).
>

done, in the draft text,  good catch.

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

maximum-packet-size appeared in 2 paragraphs, I reorganized with a small
intro.


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

 done

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

I didn't change, for me it was  not clear what to suppress. if you refer to
§3.2 it's for me an illustration of the global process.


>
> All in all, good job !
>

Thanks :-)

see you

Laurent

>
>
> 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]> 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]
> https://www.ietf.org/mailman/listinfo/lp-wan
>
>