Reviewer: Thomas Fossati
Review result: Ready with Issues
Due premise: I am not a time geek, so many of the subtleties in the document
were utterly inintellegible to me :-) E.g., the editor's note in §3.4 and the
details about the logarithmic scale in §3.5.2.
# Summary
The document defines three new CBOR tags (i.e., types) for expressing:
* time intervals
* duration, and
* richer (than RFC3339 and POSIX time) representation of time,
i.e.:
1. POSIX time with higher (than microsecond) resolution
2. explicit indication of timescale (to allow TAI alongside UTC)
The document is generally well written.
As per premise, I am not a time geek so I will refrain to comment on the
technical content.
>From an artart perspective, there are a couple of things that need fixing
before publication:
* a typo in the CDDL
* a clarification about the registration procedures of the new time tag map
registry
The rest of my comments is a bunch of editorial suggestions / nits.
## global
* s/Posix/POSIX/
* IEEE1588-2008 (PTPv2) and IEEE1588-2019 (PTPv2.1) are often cited one
alongside the other. I am not sure why is the newer, backwards
compatible IEEE1588-2019 not sufficient?
## abstract
* s/It is/This document is/
## introduction
* possibly missing ref:
* "[...] via an IANA registry ({{Section 9.2 of RFC8949}}).", or
* "[...] via an IANA registry {{IANA.cbor-tags}}."
* Given the preceding para, the sentence "In CBOR, one point of
extensibility is the definition of CBOR tags." is redundant
* s/It is/This document is/
## terminology
* the whole "Where bit arithmetic is explained, [...] exponentiation."
can be dropped.
## objectives
* "[...] for the map inside the tag" could be made more precise:
* adding an anchor to the Etime CDDL and referencing it from here, or
* saying instead "[...] for the `etime-detailed` map", or
* a combination of the two
* "[...] (Section 7.3, Section 3)." could be trimmed down to just
"(Section 7.3)"
* the bullet list with a single bullet could be converted into a normal
paragraph.
## time format
* In general, the presentation reads a bit awkward. There are a few
redundancies here and there and fluidity suffers as a consequence.
That said, nothing impacting understanding so it's not a blocking
issue, just style.
* s/Base Time/"base time"/
* there's another bullet list with a single bullet that could be
streamlined
## key 1
* In "The time value indicated by the value under this key can be
further modified by other keys.", maybe s/modified/supplemented/
* besides, I suggest adding a backpointer to §3 to restate some of the
needed context: "... as described in {{time-format}}"
## key 4 and 5
* "This can be used to include a Decimal or Bigfloat epoch-based float
[TIME_T] in an extended time."
* "... to encode higher resolution timestamps" (or similar)
## keys -3, -6, ...
* "More than one of these keys MUST NOT be present in one extended time
data item."
* reads a bit awkwardly. what about: "At most one of these keys MUST
be present [...]"
* s/compare//
## clock quality
* s/two more/the last two/
## clockclass
* "this has updated language as Table 4"
* "this uses the updated language found in Table 4"
* "(which is based on Table 5 in Section 7.6.2.4 of [IEEE1588-2008];
this has updated language as Table 4 in Section 7.6.2.5 of
[IEEE1588-2019])."
* the referenced knowledge is inaccessible to normal humans, so trying
to follow this was quite frustrating :-)
* is it really necessary to have these two IEEE citations here?
Wouldn't RFC8575 be sufficient?
* "[...] as that is the range defined there"
* can we be a bit more precise as of where?
## clockaccuracy
* Same "behind paywall" consideration as above :-(
## uncertainty
* s/extended uncertainty/expanded uncertainty/
* stray "k = 2". I guess it refers to the formula in §6.2.1 of [GUM]?
If so, add a reference.
* "[...] for a duration that represents an uncertainty [...]"
* maybe add a forward reference to §4 after "duration"
* "Implementations are free to reduce a guarantee [...]"
* not sure I understand what is meant by "reducing a guarantee" here,
and what we are telling implementers they should do
## keys -10, 10 ...
* "Keys -10 and 10 supply supplementary information, where key 10 is
critical."
* you can safely drop this as it's completely made redundant by the
following para.
## keys -11, 11 ...
* "Keys -11 and 11 supply supplementary information, where key 11 is
critical."
* ditto
## IANA time tag map registry
* "[...] and a specification reference (RFC)"
* however, the registration procedure is spec required. Is "RFC
required" what was really meant?
## period format
* In the CDDL definition of Period and clumsy-Period it looks that
`Time` should rather be `time`:
```
Period = #6.1003([
start: ~Time / null
end: ~Time / null
? duration: ~Duration / null
])
clumsy-Period = #6.1003([
(start: ~Time,
((end: ~Time,
? duration: null) //
(end: null,
duration: ~Duration))) //
(start: null,
end: ~Time,
duration: ~Duration)
```
## references
* in the note to the IEEE1588-2019 entry: s/in way that/in a way that/
--
last-call mailing list
last-call@xxxxxxxx
https://www.ietf.org/mailman/listinfo/last-call
