update: second text review #400
Conversation
core/common/src/DateTimeUnit.kt
Outdated
| * in Berlin, despite the fact that the clocks were turned back one hour, so there are, in fact, 25 hours | ||
| * between the two date-times. | ||
| * in Berlin, even though the clocks were turned back one hour, so there are, in fact, 25 hours | ||
| * between the two date/times. |
There was a problem hiding this comment.
I've looked into it, and it seems like the proper form is "datetime" with no separators: https://en.wiktionary.org/wiki/datetime
I got confused because a spell checker complained about this word.
There was a problem hiding this comment.
The problem I was trying to solve here is the inconsistency between date/time and date-time in changed files in the original PR #367. Now, I've replaced all occurrences with datetime, everywhere except for test files
There was a problem hiding this comment.
Is datetime suitable both as the noun and as the adjective? The existence of a separate adjective https://en.wiktionary.org/wiki/date-time#English leaves me with a doubt.
There was a problem hiding this comment.
Yes, it's very common for nouns to act as adjectives, modifying other nouns
If we take this specific spelling for the data type, it would be great to keep it consistent throughout the doc, especially since it's a key concept for the library
| @@ -223,7 +223,7 @@ public expect class Instant : Comparable<Instant> { | |||
| /** | |||
| * Returns the number of milliseconds from the epoch instant `1970-01-01T00:00:00Z`. | |||
| * | |||
| * Any fractional part of millisecond is rounded toward zero to the whole number of milliseconds. | |||
| * Any fractional part of a millisecond is rounded toward zero to the whole number of milliseconds. | |||
There was a problem hiding this comment.
| * Any fractional part of a millisecond is rounded toward zero to the whole number of milliseconds. | |
| * Any fractional part of the millisecond is rounded toward zero to the whole number of milliseconds. |
?
There was a problem hiding this comment.
"A" here means "any", not a particular millisecond
There was a problem hiding this comment.
Sure, but it's not an "any" millisecond we're interested in but exactly the one used as the input of this function. EDIT: for example, see the first line of this KDoc: "the number of milliseconds." Not just a number of milliseconds, but the number of milliseconds corresponding to the input.
There was a problem hiding this comment.
I disagree, the way it's written right now looks like a general rule, "Any fractional part of a millisecond is rounded toward zero"
if you want to underline the input, it should probably be "Any fractional part of the input is rounded toward zero to the whole number of milliseconds."
core/common/src/TimeZone.kt
Outdated
| @@ -176,7 +176,7 @@ public typealias ZoneOffset = FixedOffsetTimeZone | |||
| /** | |||
| * Finds the offset from UTC this time zone has at the specified [instant] of physical time. | |||
| * | |||
| * **Pitfall**: the offset returned from this function should typically not be used for datetime arithmetics, | |||
| * **Pitfall**: the offset returned from this function should typically not be used for date/time arithmetics | |||
There was a problem hiding this comment.
Probably https://en.wiktionary.org/wiki/date-time. Though I do see that this file contains the same choice of words already. Should probably be replaced to date-time and datetime throughout the whole project.
core/common/src/TimeZone.kt
Outdated
| @@ -215,7 +215,7 @@ internal expect fun Instant.toLocalDateTime(offset: UtcOffset): LocalDateTime | |||
| /** | |||
| * Finds the offset from UTC the specified [timeZone] has at this instant of physical time. | |||
| * | |||
| * **Pitfall**: the offset returned from this function should typically not be used for datetime arithmetics, | |||
| * **Pitfall**: the offset returned from this function should typically not be used for date/time arithmetics | |||
There was a problem hiding this comment.
Co-authored-by: Dmitry Khalanskiy <[email protected]>
No description provided.