dret
commented
2 years ago It would have been nice to use CDDL (RFC 8610) to defined the structures not only in text. I usually recommend it for specifications defining JSON (or CBOR) structures, because writing that down often uncovers imprecisions and questions about the definitions that don't come up otherwise.
i am working through all tasks. @hvdsomp and myself discussed this one (task 6). CDDL is not really known or used in the API community. what is getting iused is JSON schema but sadly that one is still not finished as a stable spec. so while we agree that a more formal definition would be desirable, we think right now leaving it as it is is the best solution.
richsalz
fpalombini
hvdsomp
[x] 1. -----
The following sections describe uses cases in which providing links
FP: s/uses/use
[x] 2. -----
other approaches, i.e. it is possible combine various mechanisms to
FP: s/combine/to combine
[x] 3. -----
specifically by its ABNF production rule for "Link" and subsequent
FP: Although not a strong requirement since ABNF is referenced by 8288, I suggest to add a reference to RFC 5234 to this document as well, since it appears in the text here.
[x] 4. -----
as separators but also newline characters as a means to improve usability.
FP: I am not sure I understand how newline characters improve usability. Maybe some text could be added to explain that (or if you don't think it is necessary, I'd appreciate your answer to know what I am missing).
[x] 5. -----
usability. The use of non-ASCII characters in the field value of the HTTP "Link" Header field is not interoperable.
FP: Is "interoperable" the right term here? should it not be "allowed" instead?
Section 4.2
FP: It would have been nice to use CDDL (RFC 8610) to defined the structures not only in text. I usually recommend it for specifications defining JSON (or CBOR) structures, because writing that down often uncovers imprecisions and questions about the definitions that don't come up otherwise.
[x] 7. -----
The "application/linkset+json" serialization is designed such that it can directly be used as the content of a JSON-LD serialization by adding an appropriate context. Appendix A shows an example of a possible context that, when added to a JSON serialization, allows it to be interpreted as RDF.
FP: It would be useful to add the reference to JSON-LD [W3C.REC-json-ld-20140116] that is already present in the appendix. I would also appreciate a reference to the RDF specifications. Also, the RFC editor will most likely ask you to expand those terms on first use, so I encourage you to do that already (see https://www.rfc-editor.org/materials/abbrev.expansion.txt)
Section 4.2.2
FP: nit - I would use "contain" instead of "have" when talking about members in JSON objects. A couple of examples in the text:
...
FP: I will fw this document to the I18n directorate mailing list, to see if anybody is interested to take a look at section 4.2.4.2 during Last Call. I don't think this should have many I18n comments, since this does not define new concepts but rather defines serialization for existing ones, but if anybody with I18n expertise can take another look it can't hurt.
[x] 10. -----
The Web linking model ([RFC8288]) provides for the use of extension target attributes as discussed in Section 4.2.4.3. No other form of extensions SHOULD be used. In case they are used nevertheless, they
FP: It would be good to clarify why this is only a SHOULD and not a MUST: I expect the reason for it is that there are existing cases where other forms are in fact used. In this case, it would be good to give an example and motivate the "SHOULD".
[x] 11. -----
Section 7.2 shows a client obtaining a set of links by issuing an HTTP GET on the target of the link with the "linkset" relation type, https://example.org/links/resource1.
FP: This text in section 7.3 seems redundant to me, as the reader will have already read 7.2, and IMO this text is not really needed, so I would suggest to remove it.
[x] 12. -----
voc/?show=linktypes> as was used in xref target="Response_pr_at"/>.
FP: missing a "<"
[x] 13. -----
The link relation type below should be registered by IANA per Section 6.2.1 of Web Linking [RFC8288]:
FP: I think this should point to Section 4.2 of 8288
Section 8.3 and 8.4
FP: reminder to send the media type registrations to the media-types mailing list for community review (good to do during IETF Last Call).
[x] 15. -----
context maps "application/linkset+json" representations of link sets to Dublin Core Terms. Note that the "linkset" entry in the JSON-LD
FP: I would have appreciated an informative reference for Dublin Core Terms.
Appendix B.
FP: Thank you for this, it is always interesting to see implementation status reports. Even if this will not make the final version of the document, may I suggest to please update it - in particular the following sentence makes me think this is outdated:
pertaining to products, shipments, assets and locations. Currently, the GS1 Digital Link specification makes an informative reference to version 03 of the "linkset" I-D. GS1 expresses confidence that this will become a normative reference in the next iteration of that specification.
FP: As noted by Rich in his shepherd write up, there are two references that come up as obsoleted: 6982 -> 7942 and 5988 -> 8288. Reading how they are used, I think 5988 makes sense to stay as informative. On the other hand, 6982 should not only be replaced by 7942, but I also think it should move to informative, and will most likely be removed before publication since it is only used in Appendix B which will be removed.
FP: Rich also brought up in the shepherd write up that he believes this document should have an "Update RFC 8288" header. While I see the benefit of having that sort of link in the document, I am not sure that it is necessary in this case. Hence I'd like to listen to the wg's opinion and hear if there has been a discussion about it.