(In reply to Chenxia Liu [:liuche] from comment #31) > Hmmm, who's the mentor for this bug? Are you planning to land BOTH the Phabricator patch (recently reviewed) and the Splinter patch (from 3 years ago)? My mentor is Alphan Chen. > The Splinter patch is bit-rotted and based on the deprecation plan in place here, probably doesn't need to land anymore if we're planning on removing mozPressure anyway, right? It doesn't look like whatever data you collect would change this plan. If you don't need to land the Splinter telemetry patch, please obsolete that patch so it's not confusing - then you also don't need data review because you aren't changing any telemetry. No, the splinter patch will not be landed, a moment ago I've marked it as obsolete. > > If you're planning on landing the Splinter patch though, here are few problems I see from a quick glance. Data review != code review, so you need to get this re-reviewed, and then additionally flag me for data review again: > > - Comment #14 requests a better description: "So for instance "A counter incremented every time event.mozPressure is used by a page." or "A counter incremented for each document that uses event.mozPressure."" > - The data review says this will be collected for "all release populations" - is this true? Correct me if I'm wrong, but this looks like it's NOT for release populations, but actually pre-release Yes, it should be collected all release populations (as stated in the data review: mozilla-release, all countries, all locales, no filters). It is for release populations, not for pre-release. > - The expiry for this probe has already expired so this probe would do nothing "6)" in the data review contains "for one whole release (Firefox 66), until 2019-05-14.", this is the release date of Firefox 67 (https://wiki.mozilla.org/Release_Management/Calendar). Please let me know if I misunderstood something or if the data review can be accepted and the patch can be landed.
Bug 1165211 Comment 33 Edit History
Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.
(In reply to Chenxia Liu [:liuche] from comment #31) > Hmmm, who's the mentor for this bug? Are you planning to land BOTH the Phabricator patch (recently reviewed) and the Splinter patch (from 3 years ago)? My mentor is Alphan Chen. > The Splinter patch is bit-rotted and based on the deprecation plan in place here, probably doesn't need to land anymore if we're planning on removing mozPressure anyway, right? It doesn't look like whatever data you collect would change this plan. If you don't need to land the Splinter telemetry patch, please obsolete that patch so it's not confusing - then you also don't need data review because you aren't changing any telemetry. No, the splinter patch will not be landed, a moment ago I've marked it as obsolete. > > If you're planning on landing the Splinter patch though, here are few problems I see from a quick glance. Data review != code review, so you need to get this re-reviewed, and then additionally flag me for data review again: > > - Comment #14 requests a better description: "So for instance "A counter incremented every time event.mozPressure is used by a page." or "A counter incremented for each document that uses event.mozPressure."" > - The data review says this will be collected for "all release populations" - is this true? Correct me if I'm wrong, but this looks like it's NOT for release populations, but actually pre-release Yes, it should be collected in all release populations (as stated in the data review: mozilla-release, all countries, all locales, no filters). It is for release populations, not for pre-release. > - The expiry for this probe has already expired so this probe would do nothing "6)" in the data review contains "for one whole release (Firefox 66), until 2019-05-14.", this is the release date of Firefox 67 (https://wiki.mozilla.org/Release_Management/Calendar). Please let me know if I misunderstood something or if the data review can be accepted and the patch can be landed.
(In reply to Chenxia Liu [:liuche] from comment #31) > Hmmm, who's the mentor for this bug? Are you planning to land BOTH the Phabricator patch (recently reviewed) and the Splinter patch (from 3 years ago)? My mentor is Alphan Chen. > The Splinter patch is bit-rotted and based on the deprecation plan in place here, probably doesn't need to land anymore if we're planning on removing mozPressure anyway, right? It doesn't look like whatever data you collect would change this plan. If you don't need to land the Splinter telemetry patch, please obsolete that patch so it's not confusing - then you also don't need data review because you aren't changing any telemetry. No, the splinter patch will not be landed, a moment ago I've marked it as obsolete. > > If you're planning on landing the Splinter patch though, here are few problems I see from a quick glance. Data review != code review, so you need to get this re-reviewed, and then additionally flag me for data review again: > > - Comment #14 requests a better description: "So for instance "A counter incremented every time event.mozPressure is used by a page." or "A counter incremented for each document that uses event.mozPressure."" > - The data review says this will be collected for "all release populations" - is this true? Correct me if I'm wrong, but this looks like it's NOT for release populations, but actually pre-release Yes, it should be collected in all release populations (as stated in the data review: mozilla-release, all countries, all locales, no filters). It is for release populations, not for pre-release. > - The expiry for this probe has already expired so this probe would do nothing "6)" in the data review contains "for one whole release (Firefox 66), until 2019-05-14.", this is the release date of Firefox 67 (https://wiki.mozilla.org/Release_Management/Calendar). Please let me know if I misunderstood something or if the data review can be accepted and the patch can be landed.
(In reply to Chenxia Liu [:liuche] from comment #31) > Hmmm, who's the mentor for this bug? Are you planning to land BOTH the Phabricator patch (recently reviewed) and the Splinter patch (from 3 years ago)? My mentor is Alphan Chen. > The Splinter patch is bit-rotted and based on the deprecation plan in place here, probably doesn't need to land anymore if we're planning on removing mozPressure anyway, right? It doesn't look like whatever data you collect would change this plan. If you don't need to land the Splinter telemetry patch, please obsolete that patch so it's not confusing - then you also don't need data review because you aren't changing any telemetry. No, the splinter patch will not be landed, a moment ago I've marked it as obsolete. > > If you're planning on landing the Splinter patch though, here are few problems I see from a quick glance. Data review != code review, so you need to get this re-reviewed, and then additionally flag me for data review again: > > - Comment #14 requests a better description: "So for instance "A counter incremented every time event.mozPressure is used by a page." or "A counter incremented for each document that uses event.mozPressure."" > - The data review says this will be collected for "all release populations" - is this true? Correct me if I'm wrong, but this looks like it's NOT for release populations, but actually pre-release Yes, it should be collected in all release populations (as stated in the data review: mozilla-release, all countries, all locales, no filters). It is for release populations, not for pre-release. > - The expiry for this probe has already expired so this probe would do nothing "6)" in the data review contains "for one whole release (Firefox 66), until 2019-05-14.", this is the release date of Firefox 67 (https://wiki.mozilla.org/Release_Management/Calendar) which is still in the future, hence not expired. Please let me know if I misunderstood something or if the data review can be accepted and the patch can be landed.
(In reply to Chenxia Liu [:liuche] from comment #31) > Hmmm, who's the mentor for this bug? Are you planning to land BOTH the Phabricator patch (recently reviewed) and the Splinter patch (from 3 years ago)? My mentor is Alphan Chen. > The Splinter patch is bit-rotted and based on the deprecation plan in place here, probably doesn't need to land anymore if we're planning on removing mozPressure anyway, right? It doesn't look like whatever data you collect would change this plan. If you don't need to land the Splinter telemetry patch, please obsolete that patch so it's not confusing - then you also don't need data review because you aren't changing any telemetry. No, the splinter patch will not be landed, a moment ago I've marked it as obsolete. The other patch still adds use-counters for _DOCUMENT and _PAGE (https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/telemetry/collection/use-counters.html#deprecated-dom-operations), hence the data review is still needed. > > If you're planning on landing the Splinter patch though, here are few problems I see from a quick glance. Data review != code review, so you need to get this re-reviewed, and then additionally flag me for data review again: > > - Comment #14 requests a better description: "So for instance "A counter incremented every time event.mozPressure is used by a page." or "A counter incremented for each document that uses event.mozPressure."" > - The data review says this will be collected for "all release populations" - is this true? Correct me if I'm wrong, but this looks like it's NOT for release populations, but actually pre-release Yes, it should be collected in all release populations (as stated in the data review: mozilla-release, all countries, all locales, no filters). It is for release populations, not for pre-release. > - The expiry for this probe has already expired so this probe would do nothing "6)" in the data review contains "for one whole release (Firefox 66), until 2019-05-14.", this is the release date of Firefox 67 (https://wiki.mozilla.org/Release_Management/Calendar) which is still in the future, hence not expired. Please let me know if I misunderstood something or if the data review can be accepted and the patch can be landed.