-
Notifications
You must be signed in to change notification settings - Fork 71
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
Relax return types of Eio.Process.Pipe #775
base: main
Are you sure you want to change the base?
Conversation
Hey @talex5 @patricoferris . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Arogbonlo !
I appreciate your efforts! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is good, but it also needs the same change for Eio_unix.pipe
.
Ok. I'll get that done immediately! |
Hey @talex5 @patricoferris . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to update eio_unix.mli
with the more flexible types.
Hello @talex5 , Each time i update |
In the
This union typing ([Unix_fd | Eio.Resource.close_ty | Eio.Flow.source_ty]forsource_ty) makes pipe’s output flexible enough to match what’s required in In So @talex5 , I think flexibility in |
The one in
|
I just resolved that. I just uploaded the But after doing that, the build constantly fails. |
This PR relaxes the return types of the pipe function in Eio.Process to make the code more flexible. The main goal was to allow the pipe function to handle a broader range of types for input and output flows, while still maintaining compatibility with the rest of the codebase.
Changes:
Relaxed Type Definitions: I’ve updated the return types for pipe so they can accept any subtype of
Flow.source_ty
andFlow.sink_ty
. This will give us more flexibility when working with processes that require different types of flows.Polymorphic Variants: I used polymorphic variants (
[< .. ]
) to allow for more flexible types, ensuring that the code still works with the broader variants expected in other parts of the system.Testing:
I ran dune runtest and confirmed that all tests are passing with these changes. The new types work fine, and the tests ensure everything is still functioning as expected.
This addresses issue #750, improving flexibility when working with various APIs that use stricter type definitions for sources and sinks.