As with partial-response, I really appreciate the recent editorial work on
this document: Version -14 is easy to read and mostly clear, and it’s
almost ready for last call. I have some review comments below; most are
minor, but the ones about Section 2.3 and Table 2 need to be addressed
before last call. I’ll put this into “revised I-D needed” state, and
please let me know if you have questions or disagreement with my comments
below.
— Section 1 —
operators for counting, sorting and paging rows are currently
supported by the major RDBMSs.
You use “RDBMSs” here and in Section 4, and it’s not a central term to the
document, but it should be expanded at least on first use. I suggest
simply using “relational database management systems” in both places, and
avoiding the abbreviation and having to explain it.
— Section 1.1 —
Please use the exact BCP 14 boilerplate from RFC 8174.
— Section 2.1 —
Such information is collected in two OPTIONAL response
elements named, respectively, "sorting_metadata" and
"paging_metadata".
Please remove “, respectively,” as it’s misused: there’s nothing to match
the two names up with, so no reason to use “respectively”.
o "totalCount": "Numeric" (OPTIONAL) a numeric value representing
the total number of objects found. It MUST be provided if the
query string contains the "count" parameter;
Section 2.2 says also that it MUST NOT be provided otherwise, and I suggest
adding that here as well: ‘It MUST be provided if the query string contains
the "count" parameter, and MUST NOT be provided otherwise;’, or perhaps ‘It
MUST be provided if and only if the query string contains the "count"
parameter;’. I also wonder whether the same thing is true for pageSize and
pageNumber.
This property is redundant for clients because the page size can
be derived from the length of the search results array but it can
be helpful if the end user interacts with the server through a web
browser;
But a web browser is a client too. I suggest “redundant for some clients”;
what do you think? I also suggest a comma before “but”, to break the long
sentence up a little.
— Section 2.3 —
If IP
addresses are represented as JSON strings, they MUST be sorted based
on their numeric conversion.
I think most people will understand this, but I also think it’s not clearly
specified. What does the “numeric conversion” of, say, 10.0.0.9 mean? How
does “their numeric conversion” explain that 8.8.8.8 sorts lower than
10.0.0.9 ? And I think it’s yet more complicated with v6 addresses.. I’d
like to see a *little* clearer explanation of what you mean, even though I
know what you mean and even though I think most readers will. I want to
make sure that all readers will.
— Section 2.3.1 —
identifies the root of the document (DOM).
Use your judgment on this one, because I’m not sure what the right approach
is: “document (DOM)” isn’t really right, but “document object model (DOM)”
feels unnecessary and awkward. Consider this comment and let me know what
you think is best (including leaving it as it is, or perhaps just removing
“(DOM)”).
(e.g. links, notices, remarks, etc.)
Nit: Here and elsewhere: “e.g.” and “etc.” together is redundant... you
shouldn’t have both. I suggest leaving the former and removing the
latter. Also, “e.g.” needs a comma after it, so: “(e.g., links, notices,
remarks)”.
In the first and second columns of Table 1, it is really awkward to have
the words “searchable”, “nameserver”, and “properties” split between two
lines. I think you can manage it if you make the last column one character
narrower. Also, why are there trailing “.” in column 4?
Table 2, in contrast, is extremely hard to read, because everything is
broken up onto multiple lines. It’s visual chaos. Clearly, there’s no way
to make the table work otherwise, so maybe it’s better to come up with a
different way to present the information, probably using an xml2rfc list
structure instead?
— Section 5 —
Please change the contact to “IETF” instead of “IESG”.
—
Barry
_______________________________________________
regext mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/regext
