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

Deprecate Lwt_stream.result #295

Merged
merged 1 commit into from
Dec 2, 2016
Merged

Deprecate Lwt_stream.result #295

merged 1 commit into from
Dec 2, 2016

Conversation

aantron
Copy link
Collaborator

@aantron aantron commented Nov 26, 2016

And prepare to break Lwt_stream.map_exn, which uses this type.

Users of the current map_exn will see:

Warning 3: deprecated: Lwt_stream.map_exn
This function will soon use Lwt.result instead of Lwt_stream.result.
To continue using the present signature, use Lwt_stream.Versioned.map_exn_1
To use the new variant immediately, use Lwt_stream.Versioned.map_exn_2

cc @c-cube more result meltdown :P

@aantron aantron force-pushed the stream-result branch 2 times, most recently from c50d86b to 0ed4036 Compare November 26, 2016 01:22
@Drup
Copy link
Member

Drup commented Nov 27, 2016

Lwt_stream.Versioned ? shudder a little bit.

That's not a fabulous way of making breaking change. I'm pretty sure you could just come up with a better name (wrap_all, for parallel with Lwt.wrap ?) and make the transition like that.

@vasilisp
Copy link
Contributor

@Drup, see #293 if you haven't already.

@aantron
Copy link
Collaborator Author

aantron commented Nov 27, 2016

@vasilisp, @Drup Thanks. #293 is how I intend to make breaking changes if we want to keep the name.

But, if map_exn is the wrong name for this function to begin with, fair enough. Then, if we agree on a good name for the new version (will take a look at wrap/wrap_exn later, preoccupied with something), I will only deprecate map_exn and the type and move them to the bottom of the API docs, and keep them there for a very long time. No need for .Versioned in that case.

@aantron
Copy link
Collaborator Author

aantron commented Nov 28, 2016

Renamed it to wrap_exn. There is no .Versioned module anymore.

@aantron aantron merged commit a5359e3 into master Dec 2, 2016
@aantron aantron deleted the stream-result branch December 3, 2016 12:33
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.

3 participants