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

Documentation improvements: Lwt_main.run and Lwt_io.wait_read/wait_write #547

Merged
merged 6 commits into from
Feb 13, 2018
Merged

Documentation improvements: Lwt_main.run and Lwt_io.wait_read/wait_write #547

merged 6 commits into from
Feb 13, 2018

Conversation

dmbaturin
Copy link
Contributor

Myself I've found the documentation for them confusingly incomplete and attempted to not use the former when necessary and use the latter when not necessary.

{[
let main () = Lwt_io.write_line Lwt_io.stdout "hello world"

let _ =
Copy link
Contributor

Choose a reason for hiding this comment

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

It may be better to use let () = here to encourage more a more precise definition of what's being returned.

@@ -28,6 +28,18 @@ val run : 'a Lwt.t -> 'a
then returns the value returned by the thread. If [t] fails with
an exception, this exception is raised.

Every program that uses Lwt should always use this function to launch
their threads, otherwise correct scheduling is not guaranteed.
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, this is not true when targeting Javascript via js_of_ocaml. It is true for native and bytecode compilation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Indeed, JS environments have the equivalent of Lwt_main.run built into them, conversely Lwt_main.run isn't even available when targeting JS.

@aantron aantron merged commit 19f0b11 into ocsigen:master Feb 13, 2018
@aantron
Copy link
Collaborator

aantron commented Feb 13, 2018

Thanks :)

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