Jump to conversation
Unresolved conversations (19)
@dom96 dom96 May 8, 2018
This doesn't fix my concern. The default is still `local` and that is a problem. Taking this conscious explicit choice away from the developer is a bad idea.
Outdated
lib/pure/times.nim
@skilchen skilchen May 8, 2018
i would prefer something like: ```nim type TimesTypes = DateTime | Time | Duration | TimeInterval proc `+=`*[T: TimesTypes, U](a: var T, b: U) = a = a + b proc `-=`*[T: TimesTypes, U](a: var T, b: U) = a = a - b proc `*=`*[T: TimesTypes, U](a: var T, b: U) = a = a * b ```
Outdated
lib/pure/times.nim
survivorm dom96
@skilchen skilchen May 4, 2018
sorry i forgot to tell you, that this assertion will become false, if you apply my fix for the negative nanoseconds. ``` $initDuration(milliseconds=-1500) == "-2 seconds and 500 milliseconds" ``` if nanoseconds are forced to be always positive
lib/pure/times.nim
survivorm skilchen
GULPF
@skilchen skilchen May 3, 2018
this should be: ```nim if remNS < 0: ```
Outdated
lib/pure/times.nim
survivorm
@dom96 dom96 May 3, 2018
Sorry, but please remove these. These should be put into ``system`` but for that we need a separate PR (if you could create that then I would be eternally grateful 😀)
Outdated
lib/pure/times.nim
survivorm
@dom96 dom96 May 3, 2018
Is it ever useful to do so? If not then it should just be disallowed outright.
lib/pure/times.nim
skilchen GULPF
@dom96 dom96 May 3, 2018
Docs are important :) Maybe something like this: ```nim ## Converts a duration into an array consisting of fixed time units. ## ## Each value in the array gives information about a specific unit of ## time, for example ``result[Days]`` gives a count of days. ## ## This procedure is useful for converting ``Duration`` values to strings. ```
Outdated
lib/pure/times.nim
@dom96 dom96 May 3, 2018
Please remove these comments, they might end up in the doc gen. They also don't really add much except noise. One comment is enough.
Outdated
lib/pure/times.nim
@GULPF GULPF May 3, 2018
You forgot to update the comment, it should just warn that `a + between(a, b) == b` is only guaranteed when `a` and `b` are in UTC.
Outdated
lib/pure/times.nim
survivorm
@dom96 dom96 Apr 30, 2018
Same as I've written above. Remove this function.
Outdated
lib/pure/times.nim
survivorm dom96
andreaferretti
@dom96 dom96 Apr 30, 2018
> use the format from ``format(dt: DateTime, f: string)``. Huh? I don't understand this.
Outdated
lib/pure/times.nim
dom96 survivorm
andreaferretti
@dom96 dom96 Apr 30, 2018
Why do we need this when we have ``-`` which produces a ``Duration``?
Outdated
lib/pure/times.nim
survivorm
@dom96 dom96 Apr 30, 2018
Please don't abuse runnableExamples for tests. What you have here is far too much.
Outdated
lib/pure/times.nim
survivorm
@dom96 dom96 Apr 30, 2018
``` **Warning:** ```
Outdated
lib/pure/times.nim
survivorm
@dom96 dom96 Apr 30, 2018
These kinds of operators shouldn't be defined, we need a template that does this for every type that defines a ``+``, ``-``, ``*``, etc. Otherwise defining these all the time is a PITA. I feel like I've mentioned this somewhere else but of course GitHub's search fails when searching for \`+=\` -.-
Outdated
lib/pure/times.nim
survivorm
@dom96 dom96 Apr 30, 2018
We need another proc that's a bit less granular than this proc as well. Something that will work on the Nim Forum for example: https://forum.nim-lang.org/ ("26 minutes ago", "2 hours ago").
Outdated
lib/pure/times.nim
survivorm GULPF
@dom96 dom96 Apr 30, 2018
The ``DurationParts`` type is not exported above, that's bad. I'm also wondering what these duration parts are. The documentation for this functions needs to explain this better. (Based on the procs below I have a feeling this proc is perhaps not supposed to be exported?)
Outdated
lib/pure/times.nim
survivorm
@data-man data-man Apr 27, 2018
Maybe to add ```milliseconds``` and ```microseconds```?
lib/pure/times.nim
GULPF data-man
survivorm
@data-man data-man Apr 27, 2018
Maybe change the order of the parameters? To ```proc initDuration*(weeks, days, hours, minutes, seconds, milliseconds, microseconds, nanoseconds: int64 = 0): Duration``` For compatibility with ```initDateTime```.
Outdated
lib/pure/times.nim
GULPF
Resolved conversations (0)