Skip to content

[new release] conduit-lwt-unix and conduit-async (v1.2.0)#12552

Merged
mseri merged 2 commits into
ocaml:masterfrom
samoht:release-conduit-async-v1.2.0
Sep 4, 2018
Merged

[new release] conduit-lwt-unix and conduit-async (v1.2.0)#12552
mseri merged 2 commits into
ocaml:masterfrom
samoht:release-conduit-async-v1.2.0

Conversation

@samoht
Copy link
Copy Markdown
Member

@samoht samoht commented Sep 3, 2018

CHANGES:

@camelus
Copy link
Copy Markdown
Contributor

camelus commented Sep 3, 2018

✅ All lint checks passed ed646c8
  • These packages passed lint tests: conduit-async.1.2.0, conduit-lwt-unix.1.2.0

✅ Installability check (9302 → 9304)
  • new installable packages (2): conduit-async.1.2.0 conduit-lwt-unix.1.2.0

depends: [
"jbuilder" {build & >="1.0+beta9"}
"core"
"ppx_sexp_conv"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should have the {build} tag

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@mseri no. see #11852 for more details -- ppx_sexp_conv actually ships with a runtime dependency

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh yes, you are right. I got confused because it is a build dependency below.

"jbuilder" {build & >="1.0+beta9"}
"core"
"ppx_sexp_conv"
"sexplib"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is this always needed or is it tied to ppx_sexp_conv and its need depends on the version of ppx_sexp_conv?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

it's used in the code (with Sexplib.Sexp.to_string)

depends: [
"base-unix"
"jbuilder" {build & >="1.0+beta9"}
"ppx_sexp_conv" {build}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this be a standard dependency?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

fixed, thanks

@samoht
Copy link
Copy Markdown
Member Author

samoht commented Sep 3, 2018

I've also fixed the build flags upstream: mirage/ocaml-conduit#272

@mseri
Copy link
Copy Markdown
Member

mseri commented Sep 3, 2018

The Alpine CI failure is expected, do you know if the cohttp-async.1.0.0 revdep failure is expected (logs here https://ci.ocamllabs.io/log/saved/docker-run-026b5dd972e49be5e0434d394c6eb2bb/c6ecac68a4ebdc9a7a3350a57e1e482b003e7546)?

@samoht
Copy link
Copy Markdown
Member Author

samoht commented Sep 3, 2018

No idea, but it seems that cohttp-async doesn't build fine with -safe-string. Not sure this is related to that PR; I'll have a quick look.

@mseri
Copy link
Copy Markdown
Member

mseri commented Sep 4, 2018

All the fixes have been merged. I think we can merge, but I would be great if we let the CI run again. What if we rebase the PR?

CHANGES:

* Correct depopt for conduit-lwt-unix (mirage/ocaml-conduit#260, @dra27)
* async: provide all `Async_ssl` options at config (mirage/ocaml-conduit#263, @vbmithr)
* async: add a V2 module for a new versioned API (mirage/ocaml-conduit#265, @rgrinberg)
* lwt-unix: do not link with tls.lwt on windows (mirage/ocaml-conduit#267, @samoht)
@samoht samoht force-pushed the release-conduit-async-v1.2.0 branch from e40caa0 to ed646c8 Compare September 4, 2018 10:16
@samoht
Copy link
Copy Markdown
Member Author

samoht commented Sep 4, 2018

Rebased

@mseri
Copy link
Copy Markdown
Member

mseri commented Sep 4, 2018

Thanks, now the CI is all green (with exception of the expected Alpine failure)

@mseri mseri merged commit ec75fca into ocaml:master Sep 4, 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