Make the fields of times.DateTime private
#14197
Conversation
GULPF
force-pushed
the
datetime-field-access
branch
from
8c095b9 to
cc531fc
Compare
|
The non-deprecated accessor procs should be |
lib/pure/times.nim
Outdated
| @@ -276,8 +275,7 @@ type | |||
| dSun = "Sunday" | |||
|
|
|||
| type | |||
| MonthdayRange* = range[0..31] | |||
| ## 0 represents an invalid day of the month | |||
| MonthdayRange* = range[1..31] | |||
| HourRange* = range[0..23] | |||
| MinuteRange* = range[0..59] | |||
| SecondRange* = range[0..60] | |||
There was a problem hiding this comment.
pre-existing but maybe add a comment saying that the range is 0..60 to account for leap seconds ? (I'm not even sure that's the reason, but it sure looks weird without an explanation otherwise)
There was a problem hiding this comment.
I added a comment. There's not really any good reason that the range includes 60, DateTime will never contain a leap second with the current implementation. The only difference it makes is that initDateTime allows timestamps with leap seconds
There was a problem hiding this comment.
The only difference it makes is that initDateTime allows timestamps with leap seconds
I guess that's a good enough reason for existence? if possible you could add a test that calls initDateTime with a leap second, unless it's too much work because it would fail validation as currently implemented
* Make the fields of `times.DateTime` private * PR fixes
|
FYI for posterity, this is a breaking change for code that imports the module with |
See RFC's for motivation
Closes #nim-lang/RFCs#75, refs nim-lang/RFCs#211