Skip to content
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

core(lhr): add metrics type to details #5555

Closed
wants to merge 2 commits into from
Closed

core(lhr): add metrics type to details #5555

wants to merge 2 commits into from

Conversation

paulirish
Copy link
Member

ref #5008

metricAudit.details.type // 'metric'
metricAudit.details.timespanMs // numeric value

Interestingly we don't have any code that consumes metricAudit.rawValue, so no place to have it start consuming this details.timespanMs.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

since this is a new thing and we're not really breaking anyone, I suppose it's not actually a priority before v3...

Do you think there's any risk of this changing again?

@patrickhulce patrickhulce changed the title core(lhr) add metrics type to details core(lhr): add metrics type to details Jun 25, 2018
@paulirish
Copy link
Member Author

at the moment we dont have any need for this. We'll see when it comes back up during proto-fitting

@paulirish paulirish closed this Jul 21, 2018
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