dune-release icon indicating copy to clipboard operation
dune-release copied to clipboard

Vendorable

Open lubegasimon opened this issue 3 years ago • 2 comments

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.

  • It adds dune_release_bin library and as a result, the entry point (main.ml) to dune-release is moved to bin/bin (which is a new directory).
  • It also adds opam_lint_with_cmd which is an exact implementation of the current the current opam_lint.
  • It implements a global ref, opam_lint_impl whose contents get changed (here as of now).
  • The current opam_lintfunction is changed to reference the contents of opam_lint_impl thereby leaving the current behaviour of dune-release unaffected.

lubegasimon avatar Mar 16 '22 13:03 lubegasimon

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 curl but I'm confident we'll get there.

dune-release already depends on opam libraries so I think it would make much more sense to replace the external opam lint invocation by calls to the opam libraries to run the lint in-process. This would be a much more satisfying solution than introducing global refs. I'm happy to help you with this and provide guidance if needed!

Cutting out a library to expose CLI functionality is alright by me and is somehting we already do in a bunch of tools like opam-monorepo. I would much prefer if we were to stick to the conventions we use there which is:

  • have a cli/ folder with a dune-release.cli library
  • have a bin/ folder with the main executable called dune_release.ml

It would also probably make sense to rename the library dune-release.lib but we can do that separately and as part of the 2.0.0 release.

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!

NathanReb avatar Mar 16 '22 14:03 NathanReb

Thank you for the insights, and I am happy to improve the pull request!

lubegasimon avatar Mar 17 '22 09:03 lubegasimon