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

Runtime: change representation of int64 #905

Merged
merged 6 commits into from
Jan 1, 2020
Merged

Runtime: change representation of int64 #905

merged 6 commits into from
Jan 1, 2020

Conversation

hhugo
Copy link
Member

@hhugo hhugo commented Nov 2, 2019

Before we can merge this PR,

  • we need to release 3.5.0 first

  • update packages that depend on the current representation

  • zarith_js_stubs

  • update base

@TyOverby
Copy link
Collaborator

TyOverby commented Nov 2, 2019

What is the motivation behind the change? Speed? Correctness?

@hhugo
Copy link
Member Author

hhugo commented Nov 2, 2019

Uniformity, I'd like to treat int64 as any other custom block. I'm not sure what's the impact on performance.

@hhugo hhugo added the blocked label Nov 2, 2019
@hhugo hhugo force-pushed the int64 branch 3 times, most recently from 02e5b74 to 4316341 Compare November 3, 2019 03:10
@hhugo hhugo added this to the 3.6 milestone Nov 3, 2019
@vouillon
Copy link
Member

vouillon commented Nov 8, 2019

Js_of_ocaml.Json will no longer work with these values, though...

@hhugo
Copy link
Member Author

hhugo commented Dec 18, 2019

@vouillon, I've updated the PR to restore compatibility with Js_of_ocaml.Json and ppx_deriving_json

@hhugo hhugo removed the blocked label Dec 18, 2019
@hhugo
Copy link
Member Author

hhugo commented Dec 19, 2019

@vouillon, any objections for merging this ?

@vouillon
Copy link
Member

No objection

@hhugo hhugo merged commit 887507d into master Jan 1, 2020
@hhugo hhugo deleted the int64 branch January 1, 2020 06:42
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.

None yet

3 participants