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

Merge spawn' into spawn #20

Open
Gabriella439 opened this issue Jan 18, 2014 · 4 comments
Open

Merge spawn' into spawn #20

Gabriella439 opened this issue Jan 18, 2014 · 4 comments

Comments

@Gabriella439
Copy link
Owner

spawn' is more general than spawn, but only exists as a separate function because spawn came first and I didn't want to break backwards compatibility.

Since I plan on implementing issue #14 which will be a (slightly) backwards incompatible change, I've been considering taking advantage of this version bump to also fully replace spawn with the more general spawn' function.

I'm opening this issue to solicit opinions for or against this change.

@quchen
Copy link

quchen commented Jan 18, 2014

I don't see why not. The change will create static errors, and the fix is trivial.

@PierreR
Copy link
Contributor

PierreR commented Jan 18, 2014

While working on spawn I would suggest to transpose (read, write) <- case buffer of into (write, read) to mimic the return value (output, input). This would increase the code readability because output is writing into the queue while input is reading from it.

https://github.com/Gabriel439/Haskell-Pipes-Concurrency-Library/blob/master/src/Pipes/Concurrent.hs#L174

PS: if it is OK and easier for you I can issue a pull request.

@Gabriella439
Copy link
Owner Author

Yeah, I agree that switching their order in the code makes more sense. If you write up a pull request then I will merge it.

@Gabriella439
Copy link
Owner Author

I will delay merging these for at least one release. There are lots of non-breaking changes that I can make and I would like people to preview those changes first before beginning to make breaking changes.

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

No branches or pull requests

3 participants