-
Notifications
You must be signed in to change notification settings - Fork 34
Bug 1679379 - Implement the boolean metric type #11
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
Conversation
Dexterp37
left a comment
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 took a first high level pass :) Will take a closer look after this first round.
They are unnecessary since the check-size job was disabled.
Since this is the first metric type to be implemented, I had to change a bit the database module to record and return a special MetricPayload type instead of simply a string.
1e607c8 to
4b73cbc
Compare
Dexterp37
left a comment
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 other comments. My major concern is about the requirement to pass in the Glean object to the specific metrics API.
Dexterp37
left a comment
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.
r+wc
|
@brizental can you also file a bug for adding documentation? Did we settle on adding docs to the Glean Book? |
Done https://bugzilla.mozilla.org/show_bug.cgi?id=1682785. Yes, on the MVP proposal we settled on the Glean book and I still stand by that decision, even though I am not clear on how we add Glean.js docs there. |
No description provided.