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

Unused type warning #1

Closed
wants to merge 2 commits into from
Closed

Unused type warning #1

wants to merge 2 commits into from

Conversation

mbarbin
Copy link
Owner

@mbarbin mbarbin commented May 30, 2024

This is an experimental PR to test the change made in ocaml-ppx/ppxlib#493

This is using an unreleased preview packages that combines ongoing changes from ppxlib, published here

In this repo, I don't actually need this option, rather I am creating this PR to show an live example of what happens if you supply the flag to a combination of ppx that are apparently not packaged into a standard driver. The flag is not known by the driver in use here, and thus the build fails.

I don't know yet what to think about it. On one hand I can understand that this doesn't make sense to try and supply flags if they are not going to impact the executions of said ppx, on the other hand, this makes adding the flag systematically across a code base more challenging.

@mbarbin
Copy link
Owner Author

mbarbin commented May 30, 2024

The error is visible in the ci:

 File "test/dune", line 22, characters 2-60:
22 |   (pps -unused-type-warnings=true ppx_expect ppx_sexp_value)))
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
(cd _build/.sandbox/5f610e8c3a320da8d42786e4e49d8c8d/default && .ppx/d28d5ab3a2385f4e2c080794ed7daa69/ppx.exe -unused-type-warnings=true --cookie 'library-name="letfun_test"' -o test/test__letfun.pp.mli --intf test/test__letfun.mli -corrected-suffix .ppx-corrected -diff-cmd - -dump-ast)
.ppx/d28d5ab3a2385f4e2c080794ed7daa69/ppx.exe: unknown option '-unused-type-warnings=true'.

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.

1 participant