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

Fix generator detection edge case #213

Merged
merged 1 commit into from Nov 12, 2015
Merged

Conversation

bbqsrc
Copy link
Contributor

@bbqsrc bbqsrc commented Nov 12, 2015

If you do something odd like function*(){}.bind(this), the use of .bind causes the resultant function to not have a prototype property at all. ಠ_ಠ

Note I've already incremented the patch version in the package.json.

@bbqsrc
Copy link
Contributor Author

bbqsrc commented Nov 12, 2015

I'll be investigating whether using bind with generators is ever a sane option. If it turns out it is, I'll revisit this issue.

@bbqsrc
Copy link
Contributor Author

bbqsrc commented Nov 12, 2015

Ah, don't merge this yet, going to make a minor change.

@tejasmanohar
Copy link
Member

@bbqsrc Please don't change the package.json version in a PR in the future ;) (for now, it's ok)

I should do this through a manual merge most likely and increment the version.

@bbqsrc
Copy link
Contributor Author

bbqsrc commented Nov 12, 2015

Fair enough. Will rebase it out.

@tejasmanohar
Copy link
Member

Thx, can merge the two commits into one too since it's a single fix :)

@bbqsrc
Copy link
Contributor Author

bbqsrc commented Nov 12, 2015

It's passing now. I don't know why the pull request has added your commit though. 😕

@tejasmanohar
Copy link
Member

Lol huh, did you merge something downstream or mess up cherry-pick/rebase? @bbqsrc

If you do something odd like function*(){}.bind(this), the
use of .bind causes the resultant function to not have a
prototype property *at all*. ಠ_ಠ
@bbqsrc
Copy link
Contributor Author

bbqsrc commented Nov 12, 2015

There we go. I had done git pull but forgot the --rebase

Excuse me while I go wallow in shame over here.

@tejasmanohar
Copy link
Member

haha

@tejasmanohar
Copy link
Member

but yeah this makes sense to me. nice catch

tejasmanohar added a commit that referenced this pull request Nov 12, 2015
Fix generator detection edge case
@tejasmanohar tejasmanohar merged commit a033951 into node-schedule:master Nov 12, 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.

None yet

2 participants