-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Remove {build} flags from all ppx_* dependencies. #16278
Conversation
782c571
to
26d386b
Compare
Commit: 26d386b @paurkedal has posted 32 contributions. 🌤️ opam-lint warnings 26d386b
☀️ Installability check (+0) |
It seems a bit too excessive to remove it for all ppx packages. It makes sense for ppx_sexp_lib however. |
Why is it excessive? It’s strictly correct to do so — we simply don’t know what has changed in the library that may affect the code generation phase before. |
I've started a discussion among the opam-repo maintainer team about this, and will update when we reach consensus. |
I am going to close this for now, we will reopen and update it as soon as there is a consensus. Currently, it has got stale and would need rebasing anyways |
This was suggested by @avsm in #11852, which had a more narrow scope addressing the specific issue with having
ppx_sexp_conv
as a build dependency. I agree with extending the scope, though we may want to discuss the approach before merging. I am not familiar with most of the packages involved, so what I have done here is to remove the{build}
flag on every dependency on a package starting withppx_*
, as well asbisect_ppx
andlwt_ppx
, and hoping that the CI will catch any errors.