Skip to content

Add ppx_type_conv lower bound#591

Merged
avsm merged 1 commit intomirage:masterfrom
rgrinberg:ppx_type_conv_min
Jan 6, 2018
Merged

Add ppx_type_conv lower bound#591
avsm merged 1 commit intomirage:masterfrom
rgrinberg:ppx_type_conv_min

Conversation

@rgrinberg
Copy link
Copy Markdown
Member

@rgrinberg rgrinberg commented Dec 19, 2017

Previous versions don't interact with ppx_deriving that well.

Best would be to somehow black list this version in opam-repository. But it doesn't hurt adding this here.

@hannesm

@rgrinberg
Add ppx_type_conv lower bound
9d13e67 
Previous versions don't interact with ppx_deriving that well
@hannesm
Copy link
Copy Markdown
Member

hannesm commented Dec 19, 2017 •
edited
Loading

@rgrinberg thanks! if ppx_type_conv v0.9.0 does not work anywhere (does it work anywhere?), I'd be in favour of deleting it from opam-repository!?
as far as I understand @diml, ppx_type_conv v0.9.0 needs a conflict with ppx_deriving.4.2!? once this is added, is this change here needed (well, if cohttp depends on ppx_type_conv, it should have a dependency in opam). I'm not familiar with either ppx_type_conv, ppx_deriving, nor cohttp to judge which change(s) are needed.

@rgrinberg
Copy link
Copy Markdown
Member Author

rgrinberg commented Dec 19, 2017

if ppx_type_conv v0.9.0 does not work anywhere (does it work anywhere?), I'd be in favour of deleting it from opam-repository!?

It does work for other converters that don't generate modules, since in this situation generating the same code doesn't hurt.

@ghost
Copy link
Copy Markdown

ghost commented Dec 19, 2017

Adding the conflict seems fine. Hopefully it shouldn't make packages uninstallable

@hannesm hannesm mentioned this pull request Dec 20, 2017
Merged
gasche added a commit to gasche/opam-repository that referenced this pull request Dec 20, 2017
@gasche
ppx_type_conv 0.9.0 conflicts with ppx_deriving 4.02.0
53a126c 
This is the solution to build problems affecting cohttp proposed in

  mirage/ocaml-cohttp#591
@gasche gasche mentioned this pull request Dec 20, 2017
Merged
@avsm avsm merged commit d5223f3 into mirage:master Jan 6, 2018
@avsm avsm mentioned this pull request Jan 9, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rgrinberg @hannesm @avsm