Re: [core] Iotdir last call review of draft-ietf-core-senml-etch-05

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

Thank you very much Matthias for the time spent on this extensive review.

I appreciate

-éric

On 02/09/2019, 15:09, "ietf on behalf of Matthias Kovatsch via Datatracker" <[e-mail] on behalf of [e-mail]> wrote:

    Reviewer: Matthias Kovatsch
    Review result: Ready with Nits
    
    Dear authors and list members
    
    Here is my review for draft-ietf-core-senml-etch-05 from the IoT perspective.
    
    ## Summary
    
    draft-ietf-core-senml-etch-05 defines new media types and their semantics for
    two new SenML patch document formats (JSON and CBOR, resp.). The complexity
    added to implementations that can already handle SenML is marginal and
    straight-forward. Hence, I do not see any issue for constrained devices. The
    explicit media types help in IoT scenarios, where machines communicate with
    machines.
    
    There are a few minor issues that can be solved by the authors alone. Hence, I
    marked the result as "Ready with Nits".
    
    It would be good to get the help from an expert on Windows Clipboard Formats
    and Macintosh Uniform Type Identifiers, as no good guidelines are available to
    check these IANA considerations. (This issue appears to be recurrent also for
    other specs.)
    
    ## Technical comments
    
    * P4 (3.1): I am missing assertions such as "Values in a Fetch Record MUST be
    ignored."
      * What should happen when a Patch Record does not have a value?
    
    * P5 §3: The record must not be added when the value is null. (behavior not
    described formally enough)
    
    * P7: "Windows Clipboard Name" --> Microsoft and for instance HTML spec use
    "Windows Clipboard Format"
      * Okay, the sting itself is the Windows Clipboard Format Name...
      * The long string with spaces ("SenML FETCH/PATCH format") is a bit weird for
      this purpose, no?
        * I also had the problem to find proper guidelines for Windows Clipboard
        Formats; are there any?
      * No Macintosh Uniform Type Identifier?
    
    ## Additional comment
    
    * As already discussed with one of the authors, an implication for LwM2M is
    probably that these patch documents must not be used with Executable Resources
    (one might try to execute multiple resources at once with a PATCH method). The
    application of a Patch Pack is then not idempotent anymore. Furthermore, it is
    unclear what the value should be when the LwM2M Executable Resource does not
    take arguments. * If executing multiple resources atomically is an important
    use case, I think we need another iteration to deal with the state vs RPC issue
    ("use PATCH to call function(s) without arguments by giving a new state?!")
    
    ## Editorial comments
    
    * P1 §1 (Abstract), P2 last §: "iPATCH, PATCH, and FETCH" --> "FETCH, PATCH,
    and iPATCH"
      * It is easier on the brain if the order is kept consistent...
    
    * P2 §6: "hence full name" --> "hence the unique identifiers" ?
      * RFC 8428 does not define or contain "full name", but "globally unique
      identifier for the resource"
    
    * P3 §1: "The semantics of the ..."
      * Creates question about semantics for FETCH
      * Better to reverse sentences and start with "The rest of the document uses
      the term "(i)PATCH" to refer to both methods, as the semantics of the new
      media types are the same for the CoAP PATCH and iPATCH methods."
    
    * P3 §1: ", that can be used with the" --> ", which ..."
    * P3 §3: "Also the following ..." --> to many "also", just "The following ..."
    
    * P4 §2 (3.1): "... when resolved, match resolved names" --> "identifiers"
      * names when resolved are resolved names, hence unclear what is compared
      * P2 calls them "full names"
      * See above, should be something like "globally unique identifier for the
      resource"
    * P4 §8: Add example for records with name and time
      * Would be good to quickly show what "resolved form of records" means
    * P4 §9 (3.2): Add statement that SenML patch documents are always idempotent,
    hence PATCH and iPATCH are equivalent?
      * Basically move the last sentence to the beginning and give explanation for
      "(i)PATCH".
    
    * P5 §2: "When the name" --> "When the resolved name" ?
    
    Kind regards,
    Matthias