Skip to content

Declare show's with_path argument for interface deriver as well#277

Merged
NathanReb merged 2 commits intoocaml-ppx:masterfrom
NathanReb:declare-args-in-intf
Mar 14, 2024
Merged

Declare show's with_path argument for interface deriver as well#277
NathanReb merged 2 commits intoocaml-ppx:masterfrom
NathanReb:declare-args-in-intf

Conversation

@NathanReb
Copy link
Collaborator

Even though the argument is unused in .mli files as it only affects the implementation and not the signature of the function, it used to be accepted back when declared via ppx_deriving.api so we need to keep it that way for compatibility.

Even though the argument is unused in `.mli` files as it only affects
the implementation and not the signature of the function, it used
to be accepted back when declared via `ppx_deriving.api` so we need
to keep it that way for compatibility.

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
@NathanReb
Copy link
Collaborator Author

cc @sim642.

This should fix a lot of the errors we got in ocaml/opam-repository#25458.

@sim642
Copy link
Contributor

sim642 commented Mar 12, 2024

Heh, I also started a fix: 8d6b749. Turns out one needs to bypass the value restriction to get it to compile.

Signed-off-by: Nathan Rebours <nathan.p.rebours@gmail.com>
@NathanReb
Copy link
Collaborator Author

Argh yeah just realized that! I pushed a fix just now!

@NathanReb
Copy link
Collaborator Author

@gasche do you want to give this one a look or should I go ahead and merge?

@kit-ty-kate
Copy link
Collaborator

If it fixes the packages in opam-repository I think you can just merge it

@NathanReb NathanReb merged commit 6a80fe2 into ocaml-ppx:master Mar 14, 2024
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.

3 participants