-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add support for YAML + option for non-strict mode + change sections behaviour #45
base: master
Are you sure you want to change the base?
Conversation
Unfortunately, the yaml library contains C which makes it less portable than we'd like. What about introducing a separate package for the binary that we provide? This binary can definitely introduce the yaml dependency. |
Or we make |
Optional dependencies are quite problematic and should be avoided as much as possible. We don't want our package to recompile just because the user might have chose to install the yaml packages. This might cause an unexpected build failure that might break the user's switch. |
Ok, fine. I just want YAML in the CLI, whatever solution you prefer. Do you have in mind an opam package that does that? It is not crystal clear to me how you tell dune to take the installed library and not to (re)build it. |
I was thinking the following:
|
I think that the last part of the PR ("change sections behaviour") is trying to fix #37, just like #41 and my now-merged #49. This part should not be necessary anymore. (It's impressive how many different people hit this problem and tried to fix it in different ways! I suspect that your fix may have worked, and it's less invasive than #49, but it's probably better to get rid of the list concatenation on context entry anyway.) |
@rgrinberg I support your suggestion to split the binary in a different package with other dependencies, especially if you want the core library to remain Mirage-friendly. As the master of ceremony, I would encourage you to do the split (when you have time) :-) |
Ok, I'll remove that commit. (And yes, I took the shortest path to something working :) ) |
@strub could you rebase this PR please? I'll do the package split myself. |
See issue #40.