Closed Bug 1464006 Opened 3 years ago Closed 3 years ago

Move Normandy FilterExpressions to toolkit/components/utils

Categories

(Firefox :: Normandy Client, enhancement)

57 Branch
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 62
Tracking Status
firefox62 --- fixed

People

(Reporter: leplatrem, Assigned: leplatrem)

References

Details

Attachments

(1 file)

Since Normandy is only included in browser builds, we should move the FilterExpressions module to a place that is included in other builds so that RemoteSettings can use it in Android and TB.
Blocks: 1463502
The Try builds fails on some completely unrelated jobs. I'd say we can ignore them.
Comment on attachment 8980219 [details]
Bug 1464006 - Move Normandy FilterExpressions to toolkit/components/utils

https://reviewboard.mozilla.org/r/246366/#review252586

Besides a couple comments on the tests, this seems like a straight forward move. +1

::: toolkit/components/utils/test/unit/test_FilterExpressions.js:72
(Diff revision 1)
>  
>  // Preference tests
>  add_task(async function() {
>    let val;
>    // Compare the value of the preference
> -  await SpecialPowers.pushPrefEnv({set: [["normandy.test.value", 3]]});
> +  Services.prefs.setIntPref("normandy.test.value", 3);

`SpecialPowers.pushPrefEnv` takes care of resetting prefs after the test finishes. Is this no longer needed in xpcshell tests?

::: toolkit/components/utils/test/unit/test_Sampling.js:3
(Diff revision 1)
>  "use strict";
>  
> -ChromeUtils.import("resource://normandy/lib/Sampling.jsm", this);
> +ChromeUtils.defineModuleGetter(this, "Sampling", "resource://gre/modules/components-utils/Sampling.jsm");

I suspect this should stay as `ChromeUtils.import`. Is there a reason to change to a lazy import?
Attachment #8980219 - Flags: review?(mcooper) → review+
Keywords: checkin-needed
Pushed by ncsoregi@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d51f23418197
Move Normandy FilterExpressions to toolkit/components/utils r=mythmon
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/d51f23418197
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 62
You need to log in before you can comment on or make changes to this bug.