Skip to content

[FFM-1266] metrics processor done #7

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

Merged
merged 4 commits into from
Sep 23, 2021
Merged

[FFM-1266] metrics processor done #7

merged 4 commits into from
Sep 23, 2021

Conversation

enver-bisevac
Copy link
Contributor

No description provided.

Copy link
Contributor

@knagurski knagurski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just a few suggestions 👍🏻

src/metrics.ts Outdated
const targetData: TargetData[] = [];
const metricsData: MetricsData[] = [];

for (const [, event] of Object.entries(data)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can simplify a little bit with Object.values since you don't need the key.

Suggested change
for (const [, event] of Object.entries(data)) {
for (const event of Object.values(data)) {

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a decent use-case for a reduce

@enver-bisevac enver-bisevac requested review from knagurski and a user September 23, 2021 12:06
@enver-bisevac enver-bisevac merged commit 1bc31fb into main Sep 23, 2021
@enver-bisevac enver-bisevac deleted the FFM-1266 branch September 23, 2021 12:50
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.

2 participants