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

Print an error when seeing a cram stanza and cram tests are not enabled #4245

Merged
3 commits merged into from Feb 22, 2021
Merged

Print an error when seeing a cram stanza and cram tests are not enabled #4245

3 commits merged into from Feb 22, 2021

Conversation

ghost
Copy link

@ghost ghost commented Feb 17, 2021

It is a warning for lang dune < 3.0.

I spent a good 10 minutes the other day trying to understand why dune runtest was doing nothng. Turns out it was because I had forgotten to write (cram enable) in my dune-project file. This PR fixes that.

In this case and if there is indeed a cram test, "dune runtest" does
nothing, which is bad.

Signed-off-by: Jeremie Dimino <[email protected]>
It is a wanning for lang dune < 3.0

Signed-off-by: Jeremie Dimino <[email protected]>
@ghost ghost requested a review from rgrinberg February 17, 2021 10:06
@ghost ghost mentioned this pull request Feb 17, 2021
Copy link
Member

@rgrinberg rgrinberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good warning to have. I hesitated to add it myself because we don't have a good way to disable warnings.

@ghost
Copy link
Author

ghost commented Feb 22, 2021

In that case, (cram enable) is the right way to disable the warning :)

Maybe we should make Dune not emit such warnings in vendored projects, the same way we tell the compiler to ignore warnings in vendored projects.

@ghost ghost merged commit c507979 into ocaml:main Feb 22, 2021
This pull request was closed.
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.

2 participants