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

Migrate from Jbuilder to Dune #24

Merged
merged 3 commits into from
Jun 26, 2019

Conversation

alan-j-hu
Copy link
Contributor

This commit migrates the project from Jbuilder to Dune, as Jbuilder will be
discontinued in July.

When switching from Jbuilder to Dune, I encountered warnings about record labels not listed in patterns, which I surpressed in the dune file. Do you want me to add ; _ to the record patterns to deal with the warning?

I also got a warning in examples/simple_example.ml because of an unused variable i, which I replaced with a wildcard.

Note that when I run make while using OCaml 4.08, I get deprecation notices regarding the Format module, but the package seems to install anyway. This issue seems to be unrelated to migration from Jbuilder to Dune.

This commit migrates the project from Jbuilder to Dune, as Jbuilder will be
discontinued in July.

Note that when I use OCaml 4.08.0, I get deprecation notices, yet running
`make`, then `make install`, seems to work anyway.
@mjambon
Copy link
Member

mjambon commented Jun 25, 2019

Looking great. Thank you very much.

When switching from Jbuilder to Dune, I encountered warnings about record labels not listed in patterns, which I surpressed in the dune file. Do you want me to add ; _ to the record patterns to deal with the warning?

Please do so. I think it's better to stick with default warnings as much as possible (at least in a small code base like this one).

Note that when I run make while using OCaml 4.08, I get deprecation notices regarding the Format module, but the package seems to install anyway. This issue seems to be unrelated to migration from Jbuilder to Dune.

We should fix this separately. Would you mind filing an issue for that? Otherwise I'll do it.

…bles

I also added a newline to fix a warning about an ambiguous documentation
comment, which was intended for the map function in the List module.

There are also warnings about unused names in the List module, which are
intended to shadow names from the standard library. I've disabled those
warnings for now.
@alan-j-hu
Copy link
Contributor Author

Oh yeah, sorry, I forgot to mention:

You have a List module that shadows the List module of the standard library while replacing certain functions with tail-recursive versions. However, none of these functions seem to be used in the rest of the code, and when I use Dune, I get a warning about unused names. However, if I were to remove the module, then someone might use the functions later and end up using the versions from the standard library. Should I keep or remove the module?

@mjambon
Copy link
Member

mjambon commented Jun 25, 2019

This List module isn't part of the module interface (not in the .mli), so we can do whatever we want. I see that List.rev_split is used in one spot but List.map and List.split are unused. I'd suggest removing the unused functions, and taking rev_split out of the List submodule which can now be removed.

@alan-j-hu
Copy link
Contributor Author

I have removed the List module.

@mjambon mjambon merged commit 4c34787 into ocaml-community:master Jun 26, 2019
@alan-j-hu
Copy link
Contributor Author

(Don't forget to publish the changes on OPAM.)

@mjambon
Copy link
Member

mjambon commented Jun 29, 2019

I don't have time for it. I will only do the minimum to help transfer the repo to ocaml-community or to another account or organization.

rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Aug 2, 2019
CHANGES:

- Port from jbuilder to dune. (ocaml-community/easy-format#24)

- Port to opam 2.0 and make dune a non build dependency (ocaml-community/easy-format#25)
rgrinberg added a commit to rgrinberg/opam-repository that referenced this pull request Aug 2, 2019
CHANGES:

- Port from jbuilder to dune. (ocaml-community/easy-format#24)

- Port to opam 2.0 and make dune a non build dependency (ocaml-community/easy-format#25)
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