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

Add fs2-reactive to functional-streams-for-scala #844

Closed
zainab-ali opened this issue Apr 9, 2017 · 15 comments
Closed

Add fs2-reactive to functional-streams-for-scala #844

zainab-ali opened this issue Apr 9, 2017 · 15 comments
Milestone

Comments

@zainab-ali
Copy link
Contributor

fs2-reactive is a reactive streams implementation for fs2.
It will be ready for a first release over the coming weeks.

Should this be published under the fs2 group id, and kept under the functional-streams-for-scala organization?

I am happy to be a core maintainer, but think this would help adoption and visibility.

@zainab-ali
Copy link
Contributor Author

To clarify, fs2-reactive is not ready for release. There are plenty of outstanding issues and further tests, docs and code to be added. However, these are likely to be tackled over the coming weeks.

Once these are tackled, it would be worth thinking about what to publish it under.

@rossabaker
Copy link
Member

This library is a good for the fs2 ecosystem. I don't enjoy Reactive Streams, but I enjoy the integrations it has enabled. I am glad to see this, and expect to integrate it as an optional module for http4s.

@adelbertc
Copy link
Contributor

👍 for same reasons as Ross. I am by no means a proponent of reactive streams, my interest in this is primarily motivated for work reasons. I am looking into doing hot-reloading of configuration by providing a stream of confs and my tool of choice is FS2. A Java team here would like to re-use it instead of building their own, and abstracting over the streaming portion with Reactive Streams seems useful.

@mpilquist
Copy link
Member

👍 from me. Given the stability of rx api, I'm fine with creating the fs2-reactive project as a submodule in the main fs2 repository, with the following caveats:

  • The only dependency is "org.reactivestreams" % "reactive-streams" % "1.0.0"
  • The implementation is parametric in effect type

@zainab-ali
Copy link
Contributor Author

At the moment there is an additional dependency on log4s, I'm somewhat reluctant to remove logging given the convoluted stateful nature of the reactive streams spec.

A parametric effect type would certainly be worth having - I'll test it out against the tck.

@pchiusano
Copy link
Contributor

pchiusano commented Apr 13, 2017 via email

@pchlupacek
Copy link
Contributor

I am sort of on same page like @pchiusano here. I think it will be excellent to list library in readme.md. My feeling is that (perhaps not correct) that anything that moved under fs2 repo is where all collaborators have to support such library / extension etc. Not sure if we shouldn't do this after a while library is there and we get first hands on.

@zainab-ali
Copy link
Contributor Author

Thanks for the comments.

@mpilquist I have replaced Task with a parametric effect type.
@pchiusano I have removed the dependency on log4s - the core module only depends on reactivestreams. The docs module will have a dependency on other streaming libraries (e.g. akka streams) to keep it machine testable with tut, but that won't cause problems for end users.
I will also rename this to fs2-reactive-streams for clarity.

My own preference is still to have this within the fs2 org, purely because it's useful for early adopters. All other streaming libraries come with reactivestreams support out of the box, and this is something adopters tend to look for.

However, I do think it should be a separate project. The reactivestreams spec is not something encouraged in a functional architecture.

@zainab-ali
Copy link
Contributor Author

Following the above comments, I've made a first release under com.github.zainab-ali and will reference this in the fs2 README

@mpilquist
Copy link
Member

Reopening this as fs2-reactive-streams has proven to be an integral part of the wider ecosystem -- for example, it's used in http4s. Keeping it in a separate repository has been problematic, as breakages in the FS2 main repo have to be dealt with after the fact in the fs2-reactive-streams repository. Hence, I strongly advocate for bringing this in as a submodule to the core repository for 1.0.0 and adding @zainab-ali as a contributor if desired by Zainab. Adding the submodule will increase the maintenance burden but that's intentional in this case -- we need to keep this module working as FS2 evolves.

@mpilquist mpilquist reopened this Sep 8, 2018
@mpilquist mpilquist added this to the 1.0 milestone Sep 8, 2018
@rossabaker
Copy link
Member

👍 reactive-streams is unlikely to see a major version bump and does not require Scala bumps. It is a new dependency, but is about as safe as they come.

@SystemFw
Copy link
Collaborator

SystemFw commented Sep 8, 2018

Adding the submodule will increase the maintenance burden but that's intentional in this case -- we need to keep this module working as FS2 evolves.

For all practical means, I'm already contributing heavily to reactive-streams to keep it in sync, so 👍 to this

@ngbinh
Copy link

ngbinh commented Sep 8, 2018

This is great!

@zainab-ali
Copy link
Contributor Author

👍 I'm highly in favor of it! If there are no objections, I will submit a PR to fs2 with the new module added. It's worth noting that fs2-reactive-streams has at least one transient test failure which need to be addressed before this is merged.

@mpilquist
Copy link
Member

@zainab-ali I checked with Pavel today and he's good with this too so let's go for it. I'll add you as a member now.

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

8 participants