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

Silence w70 #5104

Merged
merged 2 commits into from
Mar 24, 2022
Merged

Silence w70 #5104

merged 2 commits into from
Mar 24, 2022

Conversation

dra27
Copy link
Member

@dra27 dra27 commented Mar 23, 2022

No description provided.

Comment on lines 19 to +24
((action (run %{bin:cppo} -D "VERSION %{read-lines:version}" %{input-file})) opamVersion)))
(wrapped false))

(rule
(copy opamStubsTypes.ml opamStubsTypes.mli))

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
((action (run %{bin:cppo} -D "VERSION %{read-lines:version}" %{input-file})) opamVersion)))
(wrapped false))
(rule
(copy opamStubsTypes.ml opamStubsTypes.mli))
((action (run %{bin:cppo} -D "VERSION %{read-lines:version}" %{input-file})) opamVersion)))
(modules_without_implementation opamStubsTypes)
(wrapped false))

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think that works - this is a module without an interface?

Copy link
Member

Choose a reason for hiding this comment

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

ah right, then we just need to rename the files at the same time .ml --> .mli

Copy link
Member Author

Choose a reason for hiding this comment

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

No - that's why I didn't... that changes the library and I didn't have the courage of my convictions to do that just to silence a warning 🙂

Copy link
Member Author

Choose a reason for hiding this comment

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

(cf. the breakages in opam-file-format prior to 2.1.0!)

Comment on lines 15 to +19
(:include ../ocaml-context-flags.sexp)))
(wrapped false))

(rule
(copy opamCudfSolverSig.ml opamCudfSolverSig.mli))
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
(:include ../ocaml-context-flags.sexp)))
(wrapped false))
(rule
(copy opamCudfSolverSig.ml opamCudfSolverSig.mli))
(:include ../ocaml-context-flags.sexp)))
(modules_without_implementation opamCudfSolverSig)
(wrapped false))

Copy link
Member Author

Choose a reason for hiding this comment

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

Similarly

src/tools/dune Show resolved Hide resolved
@rjbou rjbou added this to PR in progress in Opam 2.2.0 via automation Mar 24, 2022
@rjbou rjbou added this to the 2.2.0~alpha milestone Mar 24, 2022
@rjbou rjbou merged commit ff63508 into ocaml:master Mar 24, 2022
Opam 2.2.0 automation moved this from PR in progress to Done Mar 24, 2022
@rjbou
Copy link
Collaborator

rjbou commented Mar 24, 2022

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Opam 2.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants