Last Call Review of draft-ietf-mile-jsoniodef-09
review-ietf-mile-jsoniodef-09-genart-lc-sparks-2019-07-16-00

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).