Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Time.to_sec_string example #36

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dbohdan
Copy link

@dbohdan dbohdan commented Jun 13, 2015

As of Core version 112.17.00 Time.to_sec_string requires a time zone.

@@ -147,7 +147,7 @@ let log_entry maybe_time message =
| Some x -> x
| None -> Time.now ()
in
Time.to_sec_string time ^ " -- " ^ message
Time.to_sec_string Time.Zone.local time ^ " -- " ^ message
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The right sequence should be Time.to_sec_string time Time.Zone.local

@dbohdan
Copy link
Author

dbohdan commented Jun 4, 2016

@zzhjerry Thanks for pointing that out! I've replaced the patch.

@douglarek
Copy link

douglarek commented Jul 30, 2017

utop # Time.to_sec_string ~zone:Time.Zone.local Time.now();;
Error: This function has type Time.t -> zone:Time.Zone.t -> string It is applied to too many arguments; maybe you forgot a `;'.

As of Core version 112.17.00 Time.to_sec_string requires a time zone.
`Time.Zone.local` has been removed from the package `core`. The current
version, 0.16.1, has a deprecation warning in its place. There is a
platform-specific replacement in `core-unix`. To avoid requiring
`core-unix`, let's change the time zone to UTC.
@dbohdan
Copy link
Author

dbohdan commented Aug 28, 2023

Core has undergone changes since I submitted the PR. I have updated the PR for version 0.16.1 (the version numbering scheme has changed, too).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants