-
Notifications
You must be signed in to change notification settings - Fork 124
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
Add MINIFS and MAXIFS functions #1050
Conversation
Co-authored-by: Rene Kretzschmar <rkretzschmar@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not full review yet, I just checked:
- changelog
- if the demo in CodeSandbox still works
- the docs changes in dev preview
@rkretzschmar Your general idea for the implementation of these two functions was good. I only made a few changes to comply with the standards of HyperFormula:
Once again, thanks for your contribution. You helped us a lot 🔥 |
Performance comparison of head (c9e5bb6) vs base (06047d2)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the code and it looks good. Thanks for the code reuse in ConditionalAggregationPlugin.ts
!
Codecov Report
@@ Coverage Diff @@
## develop #1050 +/- ##
========================================
Coverage 95.74% 95.74%
========================================
Files 161 161
Lines 14091 14098 +7
Branches 2974 2977 +3
========================================
+ Hits 13491 13498 +7
Misses 584 584
Partials 16 16
|
Context
@rkretzschmar in #1047 wrote:
TODO:
SumifPlugin
->ConditionalAggregationPlugin
ConditionalAggregationPlugin
ConditionalAggregationPlugin
Co-authored-by: Rene Kretzschmar rkretzschmar@gmail.com
How has this been tested?
Unit tests pass (including new test suites for MINIFS and MAXIFS
Types of changes
Related issues:
Fixes #1049
#1047
Checklist: