-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Either renaming or dropping the Sink.Calc
method
#2760
Comments
#2759 dropped the @mstoykov suggested that we should further drop the Those changes should be implemented somehow before we can consider this issue done. |
I think we can close this @oleiade ? |
I wanted to keep it open as a reminder to address calculating avg, min, max, etc "just in time" during cooldown. Which we haven't addressed yet. What do you think? |
Hmm wouldn't calculating these "just in time" actually be a performance degradation? They are currently calculated on every 👍 for closing this issue, since |
Feature Description
In the context of #2755 resolution, we have found out with @imiric that the
TrendSink.Calc
method is only implemented for theTrendSink
. Furthermore, although it is namedCalc
, its actual job is to compute the median value of the sink.Suggested Solution (optional)
We would like to propose some ideas on how to improve that state in the future by either:
TrendSink
TrendSink.Values
Median
or something along those lines (in line with theP
method), for consistency, and to explicit what it's used for.Already existing or connected issues / PRs (optional)
#2755
#2759
#2320
The text was updated successfully, but these errors were encountered: