core-wg / core-problem-details

Other
1 stars 1 forks source link

Marco's review #17

Closed thomas-fossati closed 2 years ago

thomas-fossati commented 2 years ago

https://mailarchive.ietf.org/arch/msg/core/7_Oqt82j4tl69IG6--FrSoRN8uQ/

[Section 1.1]

* Please add the usual remark "Readers are expected to be familiar with ..."

[Section 2]

* The caption of Figure 2 can better say "Concise Problem Details Data 
Item".

* The definition of the 'title' entry says: "It SHOULD NOT change from 
occurrence to occurrence of the problem."

    Do you mean "of the same problem" ?

* The definition of 'response-code' can be expanded to also point to 
Section 3 of RFC 7252, and explicitly say that the response code here is 
exactly the value of the 'Code' field in the header of the CoAP 
response. This would complement the statement and example in the last 
paragraph of the section.

* When defining the entry 'response-code', it is good to have a pointer 
also to Section 3.2 of RFC 8132, thus covering also 4.09 and 4.22.

* "Note that, unlike [RFC7807], Concise Problem Details data items have 
no explicit type."

Consistently, please consider the following changes to avoid possible 
confusion:

    - In Section 1, use "problem classes" rather than "problem types".

    - In Section 2, use "summary of the problem class" rather than 
"summary of the problem type" when defining the 'title' entry. A few 
paragraphs below, "shorthand for the category of the error" can also 
become "shorthand for the class of the error".

    - In Section 3, use "generic problem class container" rather than 
"generic problem type container".

    - In Section 5.1, as to the "Brief description" for 'title' in Table 
1, use "problem class" rather than "problem type".

[Section 3.1]

* "Consumers of a Concise Problem Details instance MUST ignore ..."

    I think it is better to use "data item" rather than "instance". That 
would be consistent with the text in other sections and would avoid 
confusion with the 'instance' entry.

[Section 3.2]

* "Consumers of a Concise Problem Details instance MUST ignore ..."

    See the comment above about using "data item" rather than "instance".

* Is it admitted to change the definition of an already existing Custom 
Problem Details entry, by updating the related documentation? Or is it 
something to discourage or even forbid, rather preferring a new entry to 
be defined altogether?

[Section 5.1]

* For 'title' and 'detail', shouldn't the CDDL Type be "text/array", 
rather than "text"? If tag 38 is used, that applies to an array, as per 
Appendix A.2.

[Nits]

* Section 3.1, s/so they never can/so they can never

* Section 3.2, s/the nested map any/the nested map, any

* Section 3.2, s/for extension that/for extensions that

* Section 3.2, s/compact representation, in/compact representation. In

* Section 3.2, s/principle, MUST NOT be/principle, it MUST NOT be

* Section 5.1, s/Entries in Standard/Entries in the Standard

* Section 5.2, s/Entries in Custom/Entries in the Custom

* Table 1/2/3, s/RFCXXXX/RFC XXXX

* Appendix B (2 instances), s/Concise Problem Details item/Concise 
Problem Details data item

* Appendix B (2 instances), s/Custom Problem Detail entry/Custom Problem 
Details entry
cabo commented 2 years ago

Closed by #18.