-
Notifications
You must be signed in to change notification settings - Fork 37
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
Vendorable #436
base: main
Are you sure you want to change the base?
Vendorable #436
Conversation
Signed-off-by: lubegasimon <[email protected]>
Signed-off-by: lubegasimon <[email protected]>
Signed-off-by: lubegasimon <[email protected]>
Signed-off-by: lubegasimon <[email protected]>
Thanks for your contribution! Getting rid of external binary dependencies is indeed something that we would like to do in dune-release and have discussed in recent dev meetings. Some parts are easier to deal with, like opam, some others slightly more involved like
Cutting out a library to expose CLI functionality is alright by me and is somehting we already do in a bunch of tools like
It would also probably make sense to rename the library It also seems from the diff in the tests that your branch needs rebasing. Finally I wish to emphasize something you are probably already aware of but making those library public doesn't mean we ensure their stability and we'd advise that ocaml-platform locks the dune-release dependency to prevent random breakage on new release of the tool! |
Thank you for the insights, and I am happy to improve the pull request! |
The goal of this PR is to modify
dune-release
in favour of ocaml-platform to not to rely on external binaries, rather use libraries.dune_release_bin
library and as a result, the entry point (main.ml
) todune-release
is moved tobin/bin
(which is a new directory).opam_lint_with_cmd
which is an exact implementation of the current the currentopam_lint
.ref
,opam_lint_impl
whose contents get changed (here as of now).opam_lint
function is changed to reference the contents ofopam_lint_impl
thereby leaving the current behaviour ofdune-release
unaffected.