-
Notifications
You must be signed in to change notification settings - Fork 98
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
Fix packaging #548
Fix packaging #548
Conversation
It seems we can't currently allocate cinaps stanzas to specific packages so I ended up also adding a test deps on cinaps in ppxlib-tools for now. I reported this upstream: ocaml/dune#11232. If there's a better, easier fix, I'll of course patch this before the release! |
Signed-off-by: Nathan Rebours <[email protected]>
a8ee1f3
to
b9b38f8
Compare
Signed-off-by: Nathan Rebours <[email protected]>
c1b380c
to
7e2e660
Compare
Not sure I understand what's wrong with the bench build, are you any more familiar with it than I am @patricoferris ? |
Signed-off-by: Nathan Rebours <[email protected]>
Ok the question rather was how did it even work before but I fixed it. It needed an explicitly declared dependency on the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @NathanReb -- sorry for only getting around to this now, post holidays. This seems like a good clean-up to me
No worries @patricoferris ! I was on holidays myself as well! |
I ran into some errors while testing out the repo before attempting the release, this should fix those:
ppxlib
so they aren't run when running something likedune build -p ppxlib-tools
oropam install -t ppxlib-tools
. This prevent some test dependencies to leak fromppxlib
toppxlib-tools
.1
, I actually ended up having to bump it to 3.8. This enabled some warnings that I guess where not part of the default build profile before and I ended up fixing those as well.