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

Bug 1658349 - Allow using the quantity metric type outside of Gecko #1198

Merged
merged 9 commits into from
Sep 14, 2020

Conversation

brizental
Copy link
Contributor

glean_parser PR: mozilla/glean_parser#225

@brizental
Copy link
Contributor Author

After #1200 I should probably drop the C# commit, but then it would have glean_parser create invalid metrics.

@Dexterp37
Copy link
Contributor

After #1200 I should probably drop the C# commit, but then it would have glean_parser create invalid metrics.

Nah, it's fine to leave this in. No worries.

Copy link
Contributor

@Dexterp37 Dexterp37 left a comment

Choose a reason for hiding this comment

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

Looks generally solid. A couple of comments, below.

glean-core/csharp/Glean/Metrics/QuantityMetricType.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@Dexterp37 Dexterp37 left a comment

Choose a reason for hiding this comment

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

r+. Let's not merge until somebody reviews the Swift part, though

Copy link
Member

@badboy badboy left a comment

Choose a reason for hiding this comment

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

This requires documentation updates here: https://mozilla.github.io/glean/book/user/metrics/quantity.html

Please also add a changelog entry.

glean-core/ios/Glean/Metrics/QuantityMetric.swift Outdated Show resolved Hide resolved
glean-core/ios/Glean/Metrics/QuantityMetric.swift Outdated Show resolved Hide resolved
Copy link
Member

@badboy badboy left a comment

Choose a reason for hiding this comment

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

Last couple of comments but then good to go.

docs/user/metrics/quantity.md Show resolved Hide resolved
docs/user/metrics/quantity.md Show resolved Hide resolved
docs/user/metrics/quantity.md Show resolved Hide resolved
@brizental brizental merged commit ccf91a7 into mozilla:main Sep 14, 2020
@brizental brizental deleted the 1658349-quantity branch September 14, 2020 11:54
badboy added a commit to badboy/application-services that referenced this pull request Sep 18, 2020
Glean changelog:

* General
  * Allow using quantity metric type outside of Gecko ([mozilla#1198](mozilla/glean#1198))
  * Update `glean_parser` to 1.28.5
    * The `SUPERFLUOUS_NO_LINT` warning has been removed from the glinter. It likely did more harm than good, and makes it hard to make metrics.yaml files that pass across different versions of `glean_parser`.
    * Expired metrics will now produce a linter warning, `EXPIRED_METRIC`.
    * Expiry dates that are more than 730 days (~2 years) in the future will produce a linter warning, `EXPIRATION_DATE_TOO_FAR`.
    * Allow using the Quantity metric type outside of Gecko.
    * New parser configs `custom_is_expired` and `custom_validate_expires` added. These are both functions that take the expires value of the metric and return a bool. (See `Metric.is_expired` and `Metric.validate_expires`). These will allow FOG to provide custom validation for its version-based `expires` values.
  * Add a limit of 250 pending ping files. ([mozilla#1217](mozilla/glean#1217)).

Note: This also gets rid of the 2 workarounds (removed code) in
AppService thanks to upstream changes.
badboy added a commit to badboy/application-services that referenced this pull request Sep 21, 2020
Glean changelog:

* General
  * Allow using quantity metric type outside of Gecko ([mozilla#1198](mozilla/glean#1198))
  * Update `glean_parser` to 1.28.5
    * The `SUPERFLUOUS_NO_LINT` warning has been removed from the glinter. It likely did more harm than good, and makes it hard to make metrics.yaml files that pass across different versions of `glean_parser`.
    * Expired metrics will now produce a linter warning, `EXPIRED_METRIC`.
    * Expiry dates that are more than 730 days (~2 years) in the future will produce a linter warning, `EXPIRATION_DATE_TOO_FAR`.
    * Allow using the Quantity metric type outside of Gecko.
    * New parser configs `custom_is_expired` and `custom_validate_expires` added. These are both functions that take the expires value of the metric and return a bool. (See `Metric.is_expired` and `Metric.validate_expires`). These will allow FOG to provide custom validation for its version-based `expires` values.
  * Add a limit of 250 pending ping files. ([mozilla#1217](mozilla/glean#1217)).

Note: This also gets rid of the 2 workarounds (removed code) in
AppService thanks to upstream changes.
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.

3 participants