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

Use %caml_bytes_set16u when it's available (with dune-configurator) #37

Merged
merged 2 commits into from
Jan 30, 2020

Conversation

dinosaure
Copy link
Member

According #36.

@dinosaure dinosaure requested review from avsm and hannesm November 28, 2019 12:11
@dinosaure
Copy link
Member Author

/cc @hhugo of course.

@dinosaure
Copy link
Member Author

Even if it's a light PPX, I don't like to put an other dependence to base64, so this PR still exists and if I did not find anything to keep compat', I will probably merge it.

@avsm
Copy link
Member

avsm commented Dec 2, 2019

For other libraries, we are simply bumping the upper bound (e.g. cstruct-unix is already 4.06+). Given that the base64 library is not changing rapidly, I'd prefer this to simply be upper bounded to 4.07+ and keep the complexity down.

I can be convinced otherwise if you feel strongly that you do want this ppx and multiversion support PR merged, however :-)

@dinosaure
Copy link
Member Author

My concerns is mostly about external projects of MirageOS where some people use base64. I discovered it when I broke compat' and renamed B64 to Base64 :( .

@dinosaure
Copy link
Member Author

Hmmhmm, I think that we can use another mechanism with dune-configurator instead to use a PPX. I will try to find a solution today.

@dinosaure dinosaure changed the title Use %caml_bytes_set16u when it's available (integrate a _light_ ppx) Use %caml_bytes_set16u when it's available (with dune-configurator) Jan 30, 2020
@dinosaure
Copy link
Member Author

I think it's a good solution and it's work for < 4.07 and >= 4.07. Merge!

@dinosaure dinosaure merged commit 44c674f into mirage:master Jan 30, 2020
dinosaure added a commit to dinosaure/opam-repository that referenced this pull request Jan 30, 2020
CHANGES:

- Remove `build` directive on dune dependency (@craigfe, mirage/ocaml-base64#35)
- Make error poly-variant open (@copy, mirage/ocaml-base64#39)
- Use `unsafe_bytes_set16u` instead `unsafe_string_set16u` (@dinosaure, @hhugo, @avsm, mirage/ocaml-base64#37)
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