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 OCaml 5 compatibility #110

Merged
merged 3 commits into from
Jul 5, 2022
Merged

Conversation

tmattio
Copy link
Collaborator

@tmattio tmattio commented Jul 5, 2022

By removing the compatibility with 4.02.3. The opam dependencies have a constraint on OCaml >= 4.08.0 anyways.

By removing the compatibility with 4.02.3. The opam dependencies have a constraint on OCaml >= 4.08.0 anyways.
@kit-ty-kate
Copy link
Contributor

kit-ty-kate commented Jul 5, 2022

Remove asciiart example which doesn't build on OCaml 5

Are you sure? it works for me

EDIT: Actually nevermind...

Comment on lines -47 to +48
ignore (Lwt_log.error ~section ~exn "callback failed with"))
Log.err (fun m -> m "callback failed with %s" (Printexc.to_string exn));
Copy link
Contributor

Choose a reason for hiding this comment

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

note that these two lines aren’t equivalent. One waits for the action to finish and the other doesn’t. I don’t know if it matter in this particular context but it’s maybe to take into account

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Right, the reason I didn't use Logs_lwt here is that it would change the signature of the function (which is exposed publicly). It would also force us to return an Lwt.t just for the logs.

I don't think it would matter here: worst case we'd see race conditions and the logs wouldn't be in the right order, but as this is just logging an exception in a widget callback, I don't think that's a problem.

@tmattio tmattio merged commit bb98730 into ocaml-community:master Jul 5, 2022
tmattio added a commit to tmattio/opam-repository that referenced this pull request Jul 5, 2022
CHANGES:

* Fix OCaml 5 compatibility (Thibaut Mattio, ocaml-community/lambda-term#110)
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.

2 participants