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 arrow function based implementation #8

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dmitriz
Copy link

@dmitriz dmitriz commented May 29, 2019

Supersedes #7

const show = console.log
, str = values([1,2,3])

show("Pulling data from the stream...")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why rename console.log? I can expect the reader to already know what console.log does, so they can understand each line, but now I had to go figure out what show was. At first I thought it was a module.

@dmitriz
Copy link
Author

dmitriz commented May 29, 2019

It is a habit thing for me, to avoid hard-coding, so I can make change in single place, e.g. to turn it silent, and also get a better performance by not repeatedly looking up the props.
But if you feel repeating console.log would be more appropriate here, I'll happily change it.

@dmitriz
Copy link
Author

dmitriz commented May 30, 2019

Maybe I need to explain the motivations behind my PR because the changes themselves are trivial.

  • I felt highly inspired by discovering the pull-streams as it really illustrates how things don't need to be bloated to be powerful :)

  • It was my understanding that this project is educational that is worth catching people's attention. Which would be helped by a fresh, even trivial update, as people often look for the last update date.

  • Right now the reader has to get through to the end for the only example.
    I've thought more examples illustrating each function might be helpful.
    But I wasn't sure about the intended scope of examples here.
    Too many examples are bloated with details not related to the phenomena they illustrate,
    while this file is nicely kept dead simple, which is not something I'd want to disturb :)

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.

2 participants