-
Notifications
You must be signed in to change notification settings - Fork 16
Generalize param mapping and allow setting arbitrary metadata in statistics-mtg2 action #104
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
base: feature/statistics-mtg2
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/statistics-mtg2 #104 +/- ##
==========================================================
Coverage ? 46.90%
==========================================================
Files ? 260
Lines ? 17395
Branches ? 1423
==========================================================
Hits ? 8159
Misses ? 9236
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
See my comments. Let's start with addressing those and then we can have another go.
} | ||
} | ||
|
||
StatisticsParamMapping* StatisticsParamMapping::instance() { |
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 wouldn't create a singleton for this just yet. We may do that later, if need be. Also, there is very specific pattern to follow when we do create a singleton. See Aggregation or Mask, for example.
d47ef57
to
c90105c
Compare
No description provided.