1139673 - De-clutter timezones extension

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Geoff Lankow (:darktrojan)]

Attachment: repackage

Comment 1

[Geoff Lankow (:darktrojan)]

I was trying to say this before accidentally submitting this bug:

This is just a collection of scattered thoughts about the timezones package. Mostly it's just moving files around so we don't have unnecessary directories everywhere.

Also there's a change to version.py in case we can't use the alphabetic version numbers on AMO. I'm not sure if that's the case or not.

Comment 2

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8572914 [details] [diff] [review]
repackage

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

::: calendar/timezones/install.rdf
@@ -35,3 @@
>      <em:description>Timezone definitions required by Lightning</em:description>
>      <em:creator>Mozilla Calendar Project</em:creator>
> -    <em:iconURL>chrome://calendar-timezones/skin/addon-icon32.png</em:iconURL>

What happened to the addon icon?

::: calendar/timezones/version.py
@@ +10,5 @@
>      data = json.load(fp)
> +    version = data["version"]
> +
> +    # Convert the alphabetic version code to numerals, for the benefit
> +    # of addons.mozilla.org, which doesn't like letters.

AMO should be fine with letters. It might try to treat it as a beta version, especially for 2015b, but there is a checkbox to make it a non-beta release.

I think you can revert this change.

Comment 3

[Geoff Lankow (:darktrojan)]

(In reply to Philipp Kewisch [:Fallen] from comment #2)
> What happened to the addon icon?

By naming it icon.png, it automatically becomes the icon, without specifying an iconURL.

> AMO should be fine with letters. It might try to treat it as a beta version,
> especially for 2015b, but there is a checkbox to make it a non-beta release.
> 
> I think you can revert this change.

Okay, I've not seen that, but I'll believe you.

Comment 4

[Geoff Lankow (:darktrojan)]

Comment on attachment 8572914 [details] [diff] [review]
repackage

Okay, I haven't got any other changes I want to make at this stage, so r? without the version.py changes.

Also, should I rename the string I changed?

Comment 5

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8572914 [details] [diff] [review]
repackage

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

r=philipp with this comment and fixed version.py

::: calendar/locales/en-US/chrome/calendar/timezones.properties
@@ +3,5 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  # extension:
>  [e-mail]=Timezone Definitions for Mozilla Calendar
> [e-mail]=Timezone definitions required by Lightning

You can't change the string name without changing the addon id. I think we'll have to just do it here without anything and notify the localizers via m.d.l10n.

::: calendar/timezones/install.rdf
@@ -35,3 @@
>      <em:description>Timezone definitions required by Lightning</em:description>
>      <em:creator>Mozilla Calendar Project</em:creator>
> -    <em:iconURL>chrome://calendar-timezones/skin/addon-icon32.png</em:iconURL>

I think I'd rather explicitly mention it instead of relying on magic filenames :)

Comment 6

[Stefan Sitter [:ssitter]]

Geoff, could you provide an updated patch with the requested changes?

Comment 7

[Geoff Lankow (:darktrojan)]

Attachment: 1139673-2.diff

I've kept icon.png as it is because it cuts out a lot of crap, which was the point of this bug. It's a well-known magic word and has existed for a long time.

Comment 8

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Comment on attachment 8659219 [details] [diff] [review]
1139673-2.diff

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

Looks good to me, r=philipp. Please make a post to mozilla.dev.l10n when landing this.

Comment 9

[Geoff Lankow (:darktrojan)]

https://hg.mozilla.org/comm-central/rev/f102a2d706b3