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

Progress on lodash upgrade #27

Closed
wants to merge 5 commits into from
Closed

Progress on lodash upgrade #27

wants to merge 5 commits into from

Conversation

justsml
Copy link
Contributor

@justsml justsml commented Sep 14, 2016

RE: #26

Can someone more familiar with the Aggregate's run code take a look at the error I'm getting in the tests?

Thanks in advance.

@justsml
Copy link
Contributor Author

justsml commented Sep 14, 2016

almost passing...

image

@kofrasa

Can you take a look and see if you can help me get the last few tests to pass?

Thanks!!!

@whitfin
Copy link

whitfin commented Sep 14, 2016

So, $max does _.max(collection, function), but Lodash's doesn't support that. You'd have to do this (I think):

var mapped = _.map(collection, function (obj) {
    return computeValue(obj, expr, null);
});

var obj = _.max(mapped);

@justsml
Copy link
Contributor Author

justsml commented Sep 15, 2016

Thanks @zackehh - that got it.

This should be good to go ❗ 💥

@kofrasa
Copy link
Owner

kofrasa commented Sep 23, 2016

Resolving since this is no longer necessary.

@kofrasa kofrasa closed this Sep 23, 2016
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

3 participants