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

Tidy up support for drop() PR #244

Merged
merged 5 commits into from
Mar 5, 2015
Merged

Conversation

svozza
Copy link
Collaborator

@svozza svozza commented Mar 3, 2015

Fixed the few minor outstanding issues on #75 to resolve #11

var s = _(function (push, next) {
push(null, 1),
push(new Error('Drop error')),
push(null, 2),
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it me, or is the spacing off around here?

tidy up error handling and tests

add no value on error test

fix docs and no value on error test

add note about error handling
@svozza svozza force-pushed the tidy-up-support-drop branch from 313aa1b to 1dbfad6 Compare March 4, 2015 01:47
@svozza
Copy link
Collaborator Author

svozza commented Mar 4, 2015

All tidied up.

@@ -2372,6 +2371,43 @@ Stream.prototype.take = function (n) {
exposeMethod('take');

/**
* Acts as the inverse of `take(n)` - instead of returning the first `n` values, it ignores the
Copy link
Collaborator

Choose a reason for hiding this comment

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

Put a link to the take docs here? [take](#take).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, makes sense given that they're not beside each other in the docs.

@vqvu
Copy link
Collaborator

vqvu commented Mar 5, 2015

OK. This looks good to me. Thanks @alexbeletsky and @svozza.

vqvu added a commit that referenced this pull request Mar 5, 2015
@vqvu vqvu merged commit b72be3d into caolan:master Mar 5, 2015
@vqvu vqvu mentioned this pull request Mar 5, 2015
@svozza svozza mentioned this pull request Apr 9, 2015
@vqvu vqvu added this to the v2.5.0 milestone Apr 17, 2015
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.

Add drop(n)
3 participants