Re: [Cbor] Review of draft-ietf-cbor-update-8610-grammar-00

[Carsten Bormann <[e-mail]>]

On 2023-12-13, at 18:10, Christian Amsüss <[e-mail]> wrote:
> 
> Hello Carsten, hello group,
> 
> as part of shepherding this document, I'm also giving the full document
> one more read.
> 
> Summarizing, it's as ready as one could expect it to be -- the items
> below are all editorial.

Thank you!

I have generated a PR with multiple commits:

https://github.com/cbor-wg/update-8610-grammar/pull/1

de2f01b * Shepherd review: streamline abstract and intro
000a88a * Shepherd review: Figure updates (add anchors, update titles, ...
23369e1 * Shepherd review: Remove 1.0/1.1/2.0 language, streamline references
5f411d8 * Shepherd review: Briefly explain "surrogate pairs"
1dfbc46 * Editor's note re https://github.com/ietf-tools/xml2rfc/issues/683
f157e60 * Remove unused BCP14 boilerplate

Details below.

Grüße, Carsten



> * Abstract: As this is currently a copy of the start of the
>  introduction, I think this could be made a bit shorter and to the
>  point.

Actually, I trimmed both, so they are now the same again (except for not using the referencing syntax in the abstract as per RFC 7322).
I also made sure abstract and intro explain how RFC 8610 is being updated.

de2f01b * Shepherd review: streamline abstract and intro

> * There is a form of elaborate consistency in the labelling of figures,
>  in that only new normative CDDL is labelled as a figure, whereas
>  illustrative old CDDL is set in a fixed width block but not accessible
>  as a figure.
> 
>  While understandable from an editing point of view, I think that this
>  convention will get across as inconsistent with readers, and suggest
>  that all blocks are turned into figures, with labels like "Previous
>  string parsing" for the block above what is now Figure 1.

I did this for the ABNF snippets, but not for the three CDDL examples.

>  ... and later in section 3, even that elaborate consistency is not
>  adhered to.

Fixed the ABNF (but not the CDDL) accordingly.

Added sourcecode-name attributes for new snippets to help kramdown-rfc-extract-sourcecode.

000a88a * Shepherd review: Figure updates (add anchors, update titles, …

> * "compatibility" pointers: Section 2 is described as targeting 1.0 and
>  2.0; the other pointers don't have such an annotation. As the
>  introduction points out that this document defines what is colloqually
>  called CDDL 1.1, the other could maybe say " Compatibility: backward
>  (not forward) (new in 1.1)"? Or something prettier than consecutive
>  parentheses, if the note on section to becomes "Compatibility: errata
>  fix; applies to 1.0 and 2.0".

I’m not sure we even need the “1.1” terminology at this point any more; I already had edited it out from the introduction before writing this...
In the end, we will reference the updated ABNF for CDDL by RFC number.

So I removed all references to 1.1 and 2.0 and replaced them by “this update” and “CDDL modules” (the latter with an informative I-D reference, as modules are not needed to allow implementing the updated syntax).

While doing this, I removed the references to the somewhat ephemeral documents:
-  I-D.draft-bormann-cbor-cddl-freezer: freezer
-  I-D.bormann-cbor-cddl-2-draft: cddl-2-draft

To do the latter, I changed the note about possibly extending bsqual in the future by pointing to edn-literal instead of cddl-2-draft.

I also removed the reference to
-  I-D.ietf-cbor-cddl-more-control: more-control

23369e1 * Shepherd review: Remove 1.0/1.1/2.0 language, streamline references

> * 2.1 (Err6527) explicitly allows Unicode surrogate pairs.

It needs them in the string syntax (only) to be like JSON strings.

> Can't change
>  that now, we're in for a dime with JSON strings. but now that it is
>  visible, for the benefit of younger readers to whom surrogate pairs
>  are a weird legacy thing, I suggest adding an explicit (greppable)
>  note that surrogate pairs are JSON's way of expressing Unicode code
>  points exceeding 0xffff. (This can also be justified by 8259 not being
>  in the terminology section, so we don't "import" "surrogate pairs").

5f411d8 * Shepherd review: Briefly explain "surrogate pairs”

I wish we could reference draft-bormann-dispatch-modern-network-unicode for more explanation, but that is mired in some less than productive discussion now.

> * Is the reference to 9165 necessarily normative? All occurrences of the
>  reference I've found are either
> 
>  * pointing at it to capture the full breadth of CDDL (abstract, 1st
>    intro)
>  * defining CDDL 1.1 (but being colloquial i doubt that's normative)
>  * referring to it in the rationale of a non-change (2.2)

Made it informative (still want to have this prominently in abstract and introduction, so people can find which documents they should be reading).

Part of
23369e1 * Shepherd review: Remove 1.0/1.1/2.0 language, streamline references


>  Conversely, 5234 sounds normative, given we're defining rules in ABNF.
>  (Maybe also 7405).

We are not using %s/%i, so I kept 7405 informative and made 5234 normative.

Also part of
23369e1 * Shepherd review: Remove 1.0/1.1/2.0 language, streamline references

> * Not complaining about the Acknowledgements still having a TODO --
>  that section has a tendency to need overhaul after IESG review anyway.

Indeed.


I also added one editor’s note in this PR:

1dfbc46 * Editor's note re https://github.com/ietf-tools/xml2rfc/issues/683

.. and removed the (currently unused) BCP14 boilerplate

f157e60 * Remove unused BCP14 boilerplate

Grüße, Carsten