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

mdx_test: Lazily initialize OCaml toplevel #376

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bcc32
Copy link

@bcc32 bcc32 commented Jun 7, 2022

This changes the OCaml toplevel to be a lazy value, which is only
forced when needed to run tests in an OCaml block.

The motivation behind this change is to avoid stderr output from the
init process (e.g., errors while registering pretty-printers) spilling
into the test executable's output, which can trip up some build tools.

@bcc32 bcc32 force-pushed the lazy-init-ocaml-toplevel branch from 901495f to 3c492e5 Compare November 22, 2022 16:49
@samoht
Copy link
Collaborator

samoht commented Dec 6, 2024

Do you mind rebasing this?

@bcc32 bcc32 force-pushed the lazy-init-ocaml-toplevel branch from 3c492e5 to f1e4c32 Compare December 10, 2024 21:15
@bcc32 bcc32 marked this pull request as draft December 10, 2024 21:55
This changes the OCaml toplevel to be a lazy value, which is only
forced when needed to run tests in an OCaml block.

The motivation behind this change is to avoid stderr output from the
init process (e.g., errors while registering pretty-printers) spilling
into the test executable's output, which can trip up some build tools.
@bcc32 bcc32 force-pushed the lazy-init-ocaml-toplevel branch from f1e4c32 to e16e8cd Compare December 10, 2024 22:05
@bcc32 bcc32 marked this pull request as ready for review December 10, 2024 22:05
@bcc32
Copy link
Author

bcc32 commented Dec 10, 2024

Done, thanks for the ping.

@samoht
Copy link
Collaborator

samoht commented Dec 11, 2024

Thanks! Do you think it would be possible to add a test case for this?

@samoht
Copy link
Collaborator

samoht commented Dec 11, 2024

Also: could you add a change entry? Many thanks!

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