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

Package pla.2.1 #22419

Merged
merged 1 commit into from
Nov 2, 2022
Merged

Package pla.2.1 #22419

merged 1 commit into from
Nov 2, 2022

Conversation

modlfo
Copy link
Contributor

@modlfo modlfo commented Oct 31, 2022

pla.2.1

Pla is a simple library and ppx syntax extension to create composable templates based on verbatim strings



🐫 Pull-request generated by opam-publish v2.1.0

"ocaml" { >= "4.07.0" }
"dune" { build & >= "2.5" }
"ppxlib" { >= "0.22.0" }
"ocaml-compiler-libs"
Copy link
Member

Choose a reason for hiding this comment

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

Are you really using ocaml-compiler-libs directly? I can only see uses of compiler-libs (those are two different libraries, one is a janestreet library, the other is a package included in OCaml itself)

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
"ocaml-compiler-libs"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is the ocaml-compiler-libs. I use it to get Location.t. However, when linking the library I need to use the name compiler-libs in the dune file. I don't know why.

Copy link
Member

Choose a reason for hiding this comment

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

That's because ppxlib uses ocaml-compiler-libs under the hood already so that makes it a transitive dependency

Copy link
Member

Choose a reason for hiding this comment

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

Do you want to change your usages of compiler-libs by ocaml-compiler-libs in your dune files and re-release? Or do that later (the dependency would be a bit misleading here though)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove it in the next revison I make to the library.


depends: [
"ocaml" { >= "4.07.0" }
"dune" { build & >= "2.5" }
Copy link
Member

Choose a reason for hiding this comment

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

Dune cannot be tagged as a build dependency (see ocaml/dune#2147)

Suggested change
"dune" { build & >= "2.5" }
"dune" { >= "2.5" }

Copy link
Member

Choose a reason for hiding this comment

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

This change got removed in your latest push-force

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have applied the suggested change.

@modlfo modlfo force-pushed the opam-publish-pla.2.1 branch 2 times, most recently from 2de7911 to 5f3c30e Compare October 31, 2022 18:10
@modlfo modlfo force-pushed the opam-publish-pla.2.1 branch from 5f3c30e to c654775 Compare November 1, 2022 07:12
@kit-ty-kate
Copy link
Member

Thanks a lot!

@kit-ty-kate kit-ty-kate merged commit c636d38 into ocaml:master Nov 2, 2022
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