Reviewer: Robert Sparks Review result: Almost Ready I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. Document: draft-ietf-mile-jsoniodef-09 Reviewer: Robert Sparks Review Date: 2019-07-16 IETF LC End Date: 2019-08-01 IESG Telechat date: Not scheduled for a telechat Summary: Almost ready for publication as a proposed standard RFC, but with minor issues and nits to address before publication I skimmed the various formal definitions. I have not verified they are complete or correct. If someone in the working group hasn't found a tool to do that, then the chairs should wrangle some intense volunteer labor to make sure these things are what you intend them to be. Minor Issues: * Section 2.2.5 says "base64 ... should be used for encoding". Why is this (a lower case) should? What happens if something doesn't base64 encode embedded raw data? * In the table in section 3.1 (and I suspect the corresponding cddl and json schema, but I haven't verified that), in the block for 'Incident', 'Indictator' is marked with a *. Looking at RFC7970, I think it should be marked with a ?. (0..1 instead of 0..*). * There are places in the prose of RFC7970 that add restrictions to conformant XML documents that aren't captured in the schema. See "An instance of one of these children MUST be present." in section 3.11 of that document for example. That constraint is not present in this document (or did I miss it)? * In section 3.2, the 7th bullet is likely an artifact of an earlier idea that was replaced by the one in the 8th bullet. I suggest deleting the 7th bullet (The one that says "Record class is replaced by RecordData class, and RecordData class is renamed to Record class." Nits: * I suggest removing 'abstract' from 'abstract IODEF JSON' in section 2. * Section 3.2 uses "CBOR/JSON IODEF" in some places (see the title and several bullets). The Introduction introduces this as just "JSON IODEF". I suggest removing the "CBOR/" wherever it occurs. * The example in Figure 6 is broken at the end. There is a BulkObservable list that says it is supposed to consist of ipv6-addr objects, but the list consists of one e-mail object. (Same happens in Figure 7 of course).
