search

janestreet / ppx_sexp_conv

Generation of S-expression conversion functions from type definitions
MIT License
88 stars 17 forks source link

Make ppx_sexp_conv compatible with ppxlib.0.18.0 #31

Closed NathanReb closed 2 years ago

NathanReb commented 4 years ago

ppxlib.0.18.0 upgrades to the 4.11 AST which results in a change in string constants representation. This PR makes ppx_sexp_conv compatible with the latest ppxlib.

You might want for the actual release of ppxlib.0.18.0 before merging this!

aalekseyev commented 4 years ago

We'll need to make this change (along with tens of others) when importing ppxlib into Jane Street. I kicked off that process. When that's done and an external release happens, all ppxes maintained by jane street will be updated together.

I'm afraid we don't have sufficient automation to merge individual PRs this change is scattered around, so this PR will not be merged.

NathanReb commented 4 years ago

Ok, I understand. Just to clarify, the point of those PRs is to cut point releases to make the JS ppx-es compatible with the newest compilers without having to wait for the next batch of JS releases.

I'd be happy to discuss how we can make this work in the future. In particular I will shortly be going through the same process for the 4.12 release and it'd be great to have this sorted out by then.

CC @jeremiedimino

ghost commented 4 years ago

Indeed. I had forgotten to mention this workflow to @aalekseyev. We just had a chat about this.

So, the plan here is that @cwong-ocaml will merge this patch in the v0.14 branch and do a minor release in the main opam repository.

NathanReb commented 4 years ago

Awesome!