807231 - Add functions to manage date preferences

Status: RESOLVED FIXED

(Cloud Services :: Firefox: Common, defect)

Description

[Gregory Szorc [:gps]]

Attachment: Functions to manage date prefs, v1

For FHR I wrote functions for savings Date instances to and from prefs. I needed to use these in multiple modules and figured they'd make a good addition to CommonUtils.

Comment 1

[Richard Newman [:rnewman]]

Comment on attachment 676912 [details] [diff] [review]
Functions to manage date prefs, v1

Review of attachment 676912 [details] [diff] [review]:
-----------------------------------------------------------------

::: services/common/utils.js
@@ +531,5 @@
> +
> +        valueInt = def;
> +      }
> +    } else {
> +      valueInt = def;

Rather than doing this, split into getEpochPref, use returns instead of variable assignments, and define getDatePref in terms of it.

I'm sure there's a use for getEpochPref, and it avoids valueInt.

@@ +556,5 @@
> +   * If the range check fails, an Error will be thrown instead of a default
> +   * value silently being used.
> +   *
> +   * @param branch
> +   *        (Preference) Branch to use to save preference in.

"Branch in which to save preference value".

Comment 2

[Gregory Szorc [:gps]]

Attachment: Functions to manage date prefs, v2

Incorporated review feedback.

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 677164 [details] [diff] [review]
Functions to manage date prefs, v2

Review of attachment 677164 [details] [diff] [review]:
-----------------------------------------------------------------

::: services/common/utils.js
@@ +506,5 @@
> +   *        (Number) The default value to use if the preference is not defined.
> +   * @param thisObj
> +   *        (Object) Object to log warnings
> +   */
> +  getEpochPref: function getDatePref(branch, pref, def=0, thisObj=null) {

Function name is wrong.

@@ +548,5 @@
> +   *        (Number) The default value (in milliseconds) if the preference is
> +   *        not defined or invalid.
> +   * @param thisObj
> +   *        (Object) Object to use for logging. If defined and it has a `_log`
> +   *        property, that Log4Moz logger will be used to record warnings.

Let's just pass in an optional `logger` instead.

Comment 4

[Gregory Szorc [:gps]]

https://hg.mozilla.org/projects/larch/rev/4ee689f6f599

Comment 5

[Richard Newman [:rnewman]]

Comment on attachment 677164 [details] [diff] [review]
Functions to manage date prefs, v2

Bulk-setting approval flags for FHR landing for FxOS ADU ping (Bug 788894).

Comment 6

[Richard Newman [:rnewman]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/77027b565e07

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/77027b565e07

Comment 8

[Alex Keybl [:akeybl]]

Comment on attachment 677164 [details] [diff] [review]
Functions to manage date prefs, v2

FHR for B2G ADU ping, won't be built/enabled for Mobile/Desktop.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/2bd6b0e38842
https://hg.mozilla.org/releases/mozilla-beta/rev/99437b69c0fc