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

Update Lib_usage.ml #1062

Merged
merged 3 commits into from
Mar 20, 2024
Merged

Update Lib_usage.ml #1062

merged 3 commits into from
Mar 20, 2024

Conversation

Halbaroth
Copy link
Collaborator

@Halbaroth Halbaroth commented Mar 19, 2024

We have changed a bit the API of the AE library but the Lib_usage.ml isn't up to date.

I don't know why I prevented Dune from compiling this file, so I restore the compilation and the test of this file.

@Halbaroth
Copy link
Collaborator Author

Halbaroth commented Mar 19, 2024

Actually, @bclement-ocp suggested to remove the compilation of examples: #796 (comment)

Do you have any reason? I can move the Lib_usage.ml file.

@bclement-ocp
Copy link
Collaborator

I admit I don't remember the details but I believe it was actually a dune configuration problem and the examples were introducing bogus dependencies on the OPAM CI; it's not a problem to build the examples in general.

I am not sure why I suggested to not build the examples; I think the issue is lack of a (package) stanza (probably (package alt-ergo-lib)).

@Halbaroth
Copy link
Collaborator Author

(executable
 (name Lib_usage)
 (public_name lib-usage)
 (package alt-ergo-lib)
 (libraries AltErgoLib)
 (modules Lib_usage)
)

?

@bclement-ocp
Copy link
Collaborator

I am not sure; I think we need to make sure that all of the following are OK (again my memory is fuzzy but if I recall correctly the examples were run for the parsers):

$ dune clean && dune build -p alt-ergo-lib @runtest
$ dune clean && dune build -p alt-ergo-parsers @runtest
$ dune clean && dune build -p alt-ergo @runtest

If all of these pass and the Lib_usage is run only in one of them (probably not parsers) then we should be fine.

We add the executable `lib_usage` into the package `alt-ergo-lib`
but we don't install it.
Copy link
Collaborator

@bclement-ocp bclement-ocp left a comment

Choose a reason for hiding this comment

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

LGTM provided that the aforementioned CLI commands succeed 👍

@Halbaroth Halbaroth merged commit b721a67 into OCamlPro:next Mar 20, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants