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

Mark Slacko as potentially compatible with Lwt 4.0 #11760

Merged
merged 1 commit into from
Apr 12, 2018

Conversation

Leonidas-from-XIV
Copy link
Contributor

According to the nice comment by @aantron in #11708 (comment) slacko fails because it expects lwt.log to exist without depending on the lwt_log message.

This came to a surprise to me since Slacko does not do any logging at all, so constraining it does not seem to be useful.

Looking in the CI log
https://ci.ocamllabs.io/log/saved/docker-run-e97b0a51ae9b04229ade4297103e6c79/e87df7e1b52e41541b179ab4024a538bfd0b1484 I can see that the error came from a module in "lwt/cohttp_lwt_unix.ml" which happens when building cohttp.0.21.1.

Therefore I would like to correct the dependency here. If any version of cohttp < 0.99 would be compatible with Lwt 4 (e.g. if someone does a maintenance release), so would slacko.

According to the nice comment by @aantron in
ocaml#11708 (comment)
slacko fails because it expects lwt.log to exist without depending on
the lwt_log message.

This came to a surprise to me since Slacko does not do any logging at
all, so constraining it does not seem to be useful.

Looking in the CI log
https://ci.ocamllabs.io/log/saved/docker-run-e97b0a51ae9b04229ade4297103e6c79/e87df7e1b52e41541b179ab4024a538bfd0b1484
I can see that the error came from a module in "lwt/cohttp_lwt_unix.ml"
which happens when building cohttp.0.21.1.

Therefore I would like to correct the dependency here. If any version of
cohttp < 0.99 would be compatible with Lwt 4 (e.g. if someone does a
maintenance release), so would slacko.
@camelus
Copy link
Contributor

camelus commented Apr 10, 2018

✅ All lint checks passed c7c48d7
  • These packages passed lint tests: slacko.0.13.0

✅ Installability check (8681 → 8681)

@aantron
Copy link
Contributor

aantron commented Apr 10, 2018

Argh, sorry about that.

Indeed, for reference of others, we also constrained a lot of old versions of cohttp, so slacko (and several other packages) may be incompatible with Lwt 4.0.0 due to a mutually-compatible cohttp not being available.

@Leonidas-from-XIV
Copy link
Contributor Author

Leonidas-from-XIV commented Apr 10, 2018 via email

@kit-ty-kate
Copy link
Member

Thanks !

@kit-ty-kate kit-ty-kate merged commit bc32eba into ocaml:master Apr 12, 2018
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.

4 participants