1116745 - mozharness apk description script: Update only a single locale

Status: RESOLVED FIXED

(Marketing :: Copy, task)

Description

[Sylvestre Ledru [:Sylvestre]]

For now, the script will update all locales.
We want to be able to update only a single (and specified) locale.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: force-locale.diff

I also took the liberty to update the code to manage better urlopen failure with better error messages.
Let me know if you prefer a separate patch for this.

Comment 2

[[e-mail] (:bhearsum)]

Comment on attachment 8544701 [details] [diff] [review]
force-locale.diff

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

::: scripts/update_apk_description.py
@@ +52,5 @@
>          }],
> +        [["--force-locale"], {
> +            "dest": "force_locale",
> +            "help": "Force a specific locale (instead of all)",
> +            "default": "all"

A better idiom for this is to set a default, and check for existence. Eg:
if self.config.get("force_locale"):
  # use it
else:
  # get the full list


Is there ever going to be a need to specify multiple locales here? If so, it might be better to deal with that now - it shouldn't be too difficult.

@@ +119,5 @@
> +        if force_locale != "all":
> +            # The user wants to update only a single locale. Let him do that.
> +            # Return an array of one element
> +            return [force_locale]
> +        else:

This method's job is to get the full list of locales - I think it would be better to account for force_locale in update_desc. Eg, an "if self.config.get("force_locale")" block around https://github.com/mozilla/build-mozharness/blob/master/scripts/update_apk_description.py#L145.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Attachment: bug_1116745.diff

Right, much better this way!  Thanks for the feedback.

> Is there ever going to be a need to specify multiple locales here? 
I don't see a use case for this (yet?). This feature is for cases like bug 1111015 (Enable Ukrainian for Description Page on Google Play)

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Merged:
https://hg.mozilla.org/build/mozharness/rev/7fed494a6a8d

I also removed some useless print:
https://hg.mozilla.org/build/mozharness/rev/0ab6a26619f1

Comment 5

[Chris Cooper [:coop] (he/him)]

In production: https://hg.mozilla.org/build/mozharness/rev/7fed494a6a8d