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

Change Stream.run to Stream.compile #1017

Closed
andrelfpinto opened this issue Dec 12, 2017 · 6 comments
Closed

Change Stream.run to Stream.compile #1017

andrelfpinto opened this issue Dec 12, 2017 · 6 comments
Milestone

Comments

@andrelfpinto
Copy link

Hi,

I don't know if there is a strong theoretical reason behind this choice, but every time someone explains Stream.run it is said that it "compiles" the Stream, nothing is really "ran" and it doesn't actually perform any of the effects.
Wouldn't it be better if it were already Stream.compile? It's, anyway, a nice analogy with a program: it is compiled and then run. Like:

stream.compile.unsafeRunSync()

Instead of:

stream.run.unsafeRunSync()
        ↓         ↳ this runs
this doesn't run
@mpilquist
Copy link
Member

I'm generally in favor of this change. My only concern is build breakage caused by changing something so fundamental -- it will impact every use of the library. Hence, if we decide to make this change, we'll need to deprecate the run methods.

I'm not sure compile is the best term to use. Consider:

  • compileLast
  • compileLog
  • compileFold
  • compileFoldMonoid
  • compileFoldSemigroup

These don't make as much sense to me. We could use the syntax namespacing approach used in other parts of FS2 (like stream.pull):

  • s.compile.void (equivalent to s.run)
  • s.compile.last (equivalent to s.runLast)
  • s.compile.toVector (equivalent to s.runLog)
  • s.compile.fold (equivalent to s.runFold)
  • s.compile.foldMonoid (equivalent to s.runFoldMonoid)
  • s.compile.foldSemigroup (equivalent to s.runFoldSemigroup)

@pchlupacek
Copy link
Contributor

I like the second approach. Not 100% bough on compile, how about build or mk ?

@mpilquist
Copy link
Member

s.interpret.toVector would be most accurate I think. I could get behind s.build.toVector too.

@SystemFw
Copy link
Collaborator

SystemFw commented Dec 14, 2017

👍 to compile or interpret plus the syntax-y thing.
I like compile better because that's what we use in the scaladoc as a "natural" way of describing what run does. Not crazy about build.

Also, although I do agree with the change, I'm not 100% sure it's worth the amount of breakage, edging towards "it is" at the moment.

@andrelfpinto
Copy link
Author

andrelfpinto commented Dec 14, 2017

Yes, I agree that it is so fundamental because it is everywhere, but on the other hand, it is not so different as changing Process to Stream, right? I also agree with deprecating the run methods.
I kinda like compile and as @SystemFw said, it is the "natural" way of describing it in the scaladocs. Even @mpilquist uses that term in his videos about fs2! About the term, when I think about what stream.build would create, it would be a stream. In my opinion compile seems better because it sounds more like a transformation into something else.
Also, if we don't use the syntax namespacing approach, we could use something as:

  • compileAndGetLast
  • compileAndLog
  • compileAndFold
  • compileAndFoldMonoid
  • compileAndFoldSemigroup

But I like the syntax namespacing suggestion of @mpilquist better.

At last, about the major breakage that would follow if changing, I think it is good to have in mind that if a change would relieve such a cognitive burden that would ease the understanding of a so fundamental part of the library, it is worth doing it. Furthermore, since we are still on a 0.x series, that would be a perfect time to make this kind of change, before it is more "set in stone" as a 1.0 release.

@mpilquist
Copy link
Member

Alright, let's go with compile and the syntax namespacing trick, resulting in methods like compile.toVector etc. We'll keep the run methods as deprecated for 0.10.

mpilquist added a commit to mpilquist/fs2 that referenced this issue Dec 20, 2017
@mpilquist mpilquist added this to the 0.10 milestone Jan 6, 2018
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

4 participants