Skip to content

Allow copy's filter to return a Promise#518

Merged
RyanZim merged 1 commit intodevelopfrom
async-filter
Nov 14, 2017
Merged

Allow copy's filter to return a Promise#518
RyanZim merged 1 commit intodevelopfrom
async-filter

Conversation

@RyanZim
Copy link
Collaborator

@RyanZim RyanZim commented Nov 9, 2017

No description provided.

Copy link
Collaborator

@manidlou manidlou left a comment

Choose a reason for hiding this comment

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

This is good @RyanZim! But, you mean we don't need to check if we have a filter opt at all? I mean calling Promise.resolve without checking whether we need to call it or not, wouldn't affect the perf for the cases that we don't have a filter opt?!

@RyanZim
Copy link
Collaborator Author

RyanZim commented Nov 9, 2017

@manidlou Good point about the perf; wouldn't have much impact if we're just calling a noop function, but with the extra Promise.resolve layer, that will probably have a perf hit.

@RyanZim RyanZim self-assigned this Nov 9, 2017
@RyanZim
Copy link
Collaborator Author

RyanZim commented Nov 14, 2017

Updated & fixed merge conflicts.

@RyanZim RyanZim requested a review from manidlou November 14, 2017 17:57
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 86.07% when pulling 9732252 on async-filter into 03fba97 on develop.

Repository owner deleted a comment from coveralls Nov 14, 2017
Copy link
Collaborator

@manidlou manidlou left a comment

Choose a reason for hiding this comment

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

LGTM.

@RyanZim RyanZim merged commit 935e189 into develop Nov 14, 2017
@RyanZim RyanZim deleted the async-filter branch November 14, 2017 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants