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.

Sign up for GitHub

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

Jump to bottom

Shutdown kind variance #50

Closed
reynir opened this issue Dec 21, 2023 · 0 comments
Closed

Shutdown kind variance #50

reynir opened this issue Dec 21, 2023 · 0 comments

Comments

@reynir
Copy link
Member

reynir commented Dec 21, 2023

The signature of shutdown is as follows:

mirage-flow/src/mirage_flow.ml

Line 39 in e115fea

val shutdown : flow -> [ `read | `write | `read_write ] -> unit Lwt.t

It would be slightly more ergonomic if the variant was [< `read | `write | `read_write ] instead as you may need coercion in some places now.

I don't think it is worth making a strictly speaking breaking change for such a small thing. We can save it for when we need to make another breaking change in the future.
hannesm added a commit to hannesm/mirage-flow that referenced this issue Feb 8, 2024
@hannesm
add coercion for shutdown variant, as suggested by @reynir in mirage#50
0f0cc66
hannesm added a commit that referenced this issue Feb 8, 2024
@hannesm
Merge pull request #52 from hannesm/variant
123e305 
add coercion for shutdown variant, as suggested by @reynir in #50
@hannesm hannesm closed this as completed Feb 8, 2024
@hannesm hannesm mentioned this issue Feb 8, 2024
Closed
hannesm added a commit to hannesm/opam-repository that referenced this issue Feb 8, 2024
@hannesm
[new release] mirage-flow (3 packages) (4.0.1)
86174d7 
CHANGES:

- move Mirage_flow.stats and pp_stats to Mirage_flow_combinators (mirage/mirage-flow#51 @hannesm)
- improve documentation of expected semantics (when write promise is resolved,
  what is done to the underlying flow - addresses mirage/mirage-flow#4 @samoht),
  (mirage/mirage-flow#51 @reynir @dinosaure @hannesm)
- add < coercion to shutdown:
  ``shutdown : flow -> [< `read | `write | `read_write ] -> unit Lwt.t``
  (requested mirage/mirage-flow#50 @reynir, mirage/mirage-flow#52 @hannesm)
hannesm added a commit to hannesm/opam-repository that referenced this issue Feb 8, 2024
@hannesm
[new release] mirage-flow (3 packages) (4.0.1)
b03fc14 
CHANGES:

- move Mirage_flow.stats and pp_stats to Mirage_flow_combinators (mirage/mirage-flow#51 @hannesm)
- improve documentation of expected semantics (when write promise is resolved,
  what is done to the underlying flow - addresses mirage/mirage-flow#4 @samoht),
  (mirage/mirage-flow#51 @reynir @dinosaure @hannesm)
- add < coercion to shutdown:
  ``shutdown : flow -> [< `read | `write | `read_write ] -> unit Lwt.t``
  (requested mirage/mirage-flow#50 @reynir, mirage/mirage-flow#52 @hannesm)
hannesm added a commit that referenced this issue Feb 8, 2024
@hannesm
Revert "add coercion for shutdown variant, as suggested by @reynir in #…
709ea6d 
…50"

This reverts commit 0f0cc66.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@hannesm @reynir