721165 - Extract repeated code for retrieving CHANNEL information from Blocklist and Telemetry to a javascript module

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Mark Reid [:mreid]]

There is duplicate code in 
toolkit/mozapps/extensions/nsBlocklistService.js
toolkit/components/telemetry/TelemetryPing.js
toolkit/components/urlformatter/nsURLFormatter.js

for "getUpdateChannel()" function and possibly elsewhere.  See bug 720785 for further discussion.

Comment 1

[Andres Hernandez [:andreshm]]

I'll take a look at this.

Comment 2

[Andres Hernandez [:andreshm]]

Attachment: Patch v1

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 659758 [details] [diff] [review]
Patch v1

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

::: toolkit/content/UpdateChannel.jsm
@@ +13,5 @@
> +   * Read the update channel from defaults only.  We do this to ensure that
> +   * the channel is tightly coupled with the application and does not apply
> +   * to other instances of the application that may use the same profile.
> +   */
> +  get: function UpdateChannel_get() {

Please choose a more descriptive name for this method, like getDefaultUpdateChannel or getDefaultChannel.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(In reply to Drew Willcoxon :adw from comment #3)
> > +  get: function UpdateChannel_get() {
> 
> Please choose a more descriptive name for this method, like
> getDefaultUpdateChannel or getDefaultChannel.

Isn't UpdateChannel.getDefaultUpdateChannel() redundant? UpdateChannel.get() seems pretty clear self-explanatory.

Comment 5

[Andres Hernandez [:andreshm]]

I agree that is clear UpdateChannel.get(), but if in the future there can be more getters then maybe we can use UpdateChannel.getDefault()?

Pushed to try: https://tbpl.mozilla.org/?tree=Try&rev=17d15b1c5f74

Comment 6

[Andres Hernandez [:andreshm]]

All green :) https://tbpl.mozilla.org/?tree=Try&rev=17d15b1c5f74

Comment 7

[:Gavin Sharp [email: [e-mail]]]

I don't think there will be a need for future additional getters, this module is very simple.

Comment 8

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/integration/fx-team/rev/6205f156d432

Comment 9

[Tim Taubert [:ttaubert] (inactive)]

https://hg.mozilla.org/mozilla-central/rev/6205f156d432