Skip to content

refactor(user.model.spec): reduce callback hell #1150

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

Closed
wants to merge 2 commits into from

Conversation

deltreey
Copy link

by passing functions and reducing function wrapping we can keep complexity low and make
everything cleaner and more readable

this is a small change, just the user model tests. If you guys like this style I can expand upon it, either making this PR larger or by making another. Let me know. 👍

by passing functions and reducing function wrapping we can keep complexity low and make
everything cleaner and more readable
return User.removeAsync();
});
// Clear users before testing
before(User.removeAsync);
Copy link
Member

Choose a reason for hiding this comment

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

@deltreey my apologies, I should have mentioned that the User methods will lose context to their parent object, so they'll need to be bound like: User.removeAsync.bind(User)

Copy link
Member

Choose a reason for hiding this comment

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

Babel supports ::User.removeAsync too, right?

mongoose methods lose their context if you don't bind them
@deltreey
Copy link
Author

this PR seems to be broken, I'm testing a bit, seeing if travis and my local box are not agreeing on stuff. hold off on merging this please :)

@Awk34 Awk34 added this to the 3.1.0 milestone Aug 19, 2015
@Awk34
Copy link
Member

Awk34 commented Dec 19, 2015

I wouldn't really describe this as "callback hell", and could argue that this PR would add more complication

@Awk34 Awk34 closed this Dec 19, 2015
@deltreey
Copy link
Author

I agree. Should've abandoned this pull req a while ago

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