Add metric extraction utility methods to Context#227
Conversation
danielkza
force-pushed
the
context_metrics_methods
branch
from
60bc710 to
2d8f9c9
Compare
| end | ||
| end | ||
|
|
||
| def tree_metrics |
There was a problem hiding this comment.
Where are we using this method?
There was a problem hiding this comment.
It's not used right now but it will be used immediately when I make the PR for the interpreting rewrite.
|
Good refactoring! Congratulations! |
| end | ||
|
|
||
| def tree_native_metrics | ||
| native_metrics.values.flat_map do |metrics| |
There was a problem hiding this comment.
Do you think an ||= would help here?
There was a problem hiding this comment.
I don't think it's necessary since this method should not be a performance problem at all: we never have more than 2 digits of metrics.
|
Any other comments? When this is merge I can make the interpreting optimization PR. |
|
For me it's good to go. @mezuro/core ? |
|
Would you mind if I split this commit into smaller ones by Friday? |
|
Not at all. I can do it today if you tell me what you think should be split. |
|
I was thinking about:
What do you think? |
3 participants
Code for selecting some metrics among those stored in a context was repeated in multiple places. Extract it to methods in preparation for the interpreting processing step rewrite (can also be incorporated to the aggregation rewrite in #225).