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 1609968 - Remove locale from baseline ping #1016

merged 3 commits into from Jun 30, 2020


Copy link

@brizental brizental commented Jun 25, 2020

Fixes Bug 1609968

Opening this as a draft because I believe the send_if_empty addition to the baseline ping should be discussed.

Removing the locale means that when foreground or dirty_startup the baseline ping will have no metrics, so I believe adding this flag makes sense. If everyone agrees on that, still this would need to wait for a fix in the glean_parser, because the send_if_empty breaks the baseline ping documentation.

@brizental brizental requested review from Dexterp37 and mdboom and removed request for Dexterp37 Jun 25, 2020
Copy link

@Dexterp37 Dexterp37 left a comment

Is this intentionally a draft?

Copy link
Member Author

brizental commented Jun 25, 2020

Is this intentionally a draft?

Yeah, was not sure about the send_if_empty and even if you all are okay with that this breaks the baseline pings documentation due to a bug in the glean_parser, would need to fix that before merge.

UPDATE: opened a bug for the glean_parser problem, will fix that and then we can merge this after.

@brizental brizental force-pushed the 1609968-remove-locale-baseline branch from 8061940 to 4e93618 Compare Jun 30, 2020
@brizental brizental marked this pull request as ready for review Jun 30, 2020
@brizental brizental requested review from Dexterp37 and removed request for mdboom Jun 30, 2020
@brizental brizental merged commit a259deb into mozilla:main Jun 30, 2020
32 of 33 checks passed
@brizental brizental deleted the 1609968-remove-locale-baseline branch Jun 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

None yet

4 participants