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 Toploop.load_file for 4.14.0+trunk #50

Closed

Conversation

shakthimaan
Copy link

4.14.0+trunk has deprecated Toploop.use_file and we now need to use Toploop.load_file.

@dbuenzli
Copy link
Owner

dbuenzli commented Aug 5, 2021

I guess this is issue #49. I'm a bit surprised by this. In the toplevel there is quite a difference between loading a file and using it. Could you please point to the discussion were this happened ?

Also does the fix work in earlier versions ?

@shakthimaan
Copy link
Author

shakthimaan commented Aug 5, 2021

I guess this is issue #49.

Yes.

Could you please point to the discussion were this happened ?

I referred the Toploop.mli sources.
Source: https://github.com/ocaml/ocaml/blob/a208a29bcc60ff336fc21a96b0a8a128197d5493/toplevel/toploop.mli

Also does the fix work in earlier versions ?

No. This is a breaking change.

@dbuenzli
Copy link
Owner

dbuenzli commented Aug 5, 2021

I referred the Toploop.mli sources.

Do you actually understand what use_file did and what load_file does ?

No. This is a breaking change.

Well I'm sorry but there's no way I can merge this.

@shakthimaan
Copy link
Author

Well I'm sorry but there's no way I can merge this.

Can you please suggest an alternative?

@dbuenzli
Copy link
Owner

dbuenzli commented Aug 5, 2021

Can you please suggest an alternative?

See the discussion in ocaml/ocaml#10556.

A more general comment, your help is appreciated, but before making PRs, especially a massive amount of them, please make sure you understand what you do and/or coordinate with me.

@dbuenzli dbuenzli closed this Aug 5, 2021
@shakthimaan
Copy link
Author

... and/or coordinate with me.

Since you had mentioned (ocaml-bench/sandmark#250 (comment)) that you didn't have the time, I had created the PR for you to take a look when you can actually get to it. But, thanks for the quick and prompt follow-up. Appreciate it!

I shall coordinate with you in future before making PRs. Thanks again!

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