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

Only call groupBy for actions that aren't filtered #206

Merged
merged 1 commit into from
Jul 2, 2018

Conversation

levilansing
Copy link
Contributor

I'd like to propose a very small behavior change before the big 1.0:

Currently, groupBy is called for every action, even actions that are filtered. This change would only call groupBy when necessary, specifically, only for actions that are not filtered and will affect the history.

This is more of a logical/contractual change than anything else, but it would provide a small performance boost in the case of complex grouping logic. I'm proposing this change mainly because I was surprised groupBy was called for every action, and then I realized groupBy has no idea if the action was filtered or not, so I had to move some of my grouping logic into the filter option.

@omnidan
Copy link
Owner

omnidan commented Jul 2, 2018

thanks for the PR! looks good to me 😁

@omnidan omnidan merged commit 0525ee0 into omnidan:master Jul 2, 2018
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