Closed paurkedal closed 6 years ago
I think the way you implemented it in the PR is great. Thanks for the feature!
I realise only now that this PR breaks backwards compatibility. The issue is the the a mandatory named argument does not cancel previous optional arguments like a unnamed argument would do,
utop # let f ?(a = 0) ~b = a + b;;
val f : ?a:int -> b:int -> int = <fun>
utop # f ~b:3 = 2;;
Error: This expression has type int but an expression was expected of type
?a:int -> int
That means making the argument optional also does not serve much purpose, as one might as well write ~microsecond:0
as ?microsecond:None
. I am sorry for the oversight.
The question then is how to resolve it, assuming you agree that the current signature is not optimal. The most common practise, as far as I have seen, is to add at least one unnamed argument, even if it is unit. My suggestion would be the straight forward adjustment
val datetime : year: int -> month: int -> day: int -> hour: int -> minute: int -> second: int -> ?microsecond: int -> unit -> t
Another option would be to make the microsecond
mandatory. I can send a new PR, but I'd like to know you preference first, and also whether the date
and time
should be adjusted for consistency.
Petter, I think the extra parameter is the way to go, along with keeping everything consistent in date
and time
.
Good I created #23.
According to the MySQL 8.0 documentation,
second_part
field represents microseconds, so presumably this is set in stone, but if you prefer, we could keep thesecond_part
name in OCaml, useint64
instead ofint
and exposeSEC_PART_DIGITS
.I had to put
?microsecond
before~second
to make it truly optional without fully breaking backwards compatibility. Alternatively we could add_us
variants to to all functions taking~second
arguments.