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

Rename EffectHandlers module to Effect #10879

Merged
merged 3 commits into from
Jan 12, 2022

Conversation

avsm
Copy link
Member

@avsm avsm commented Jan 11, 2022

This makes the name more consistent with the rest of the stdlib, and neatly avoids a choice of snake or camelcase. Discussed on caml-devel in December and one of the post-multicore-merge tasks in #10861.

Will require some changes in domainslib/eio to follow once merged, and in opam-repository in the base-effects package.

@nojb
Copy link
Contributor

nojb commented Jan 11, 2022

Is a bootstrap necessary?

@avsm
Copy link
Member Author

avsm commented Jan 11, 2022

It failed without it, but that might have been due to some lingering dependency. Let me drop the bootstrap with a clean tree and try again.

avsm added 3 commits January 11, 2022 15:36
This makes the name more consistent with the rest of the stdlib,
and neatly avoids a choice of snake or camelcase.  Discussed
on caml-devel in December.
@avsm avsm force-pushed the rename-effecthandlers-module branch from 8ed4ceb to 05ce8fa Compare January 11, 2022 15:40
@avsm
Copy link
Member Author

avsm commented Jan 11, 2022

And it works fine without bootstrap with a git clean -dxf, as the compiler doesn't use effect handlers (yet!). I've dropped that from the PR.

Copy link
Member

@damiendoligez damiendoligez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@nojb nojb merged commit f3f6ee0 into ocaml:trunk Jan 12, 2022
OlivierNicole added a commit to OlivierNicole/domainslib that referenced this pull request Jan 17, 2022
@avsm avsm deleted the rename-effecthandlers-module branch February 9, 2022 12:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants