De-clutter timezones extension

RESOLVED FIXED in 4.8

Status

Calendar
General
RESOLVED FIXED
3 years ago
2 years ago

People

(Reporter: darktrojan, Assigned: darktrojan)

Tracking

unspecified

Details

(Whiteboard: [timezone])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

3 years ago
Created attachment 8572914 [details] [diff] [review]
repackage
Attachment #8572914 - Flags: feedback?(philipp)
(Assignee)

Comment 1

3 years ago
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.
(Assignee)

Updated

3 years ago
Assignee: nobody → geoff
Status: NEW → ASSIGNED
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.
Attachment #8572914 - Flags: feedback?(philipp) → feedback+
(Assignee)

Comment 3

3 years ago
(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.
(Assignee)

Comment 4

3 years ago
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?
Attachment #8572914 - Flags: review?(philipp)
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:
>  extensions.calendar-timezones@mozilla.org.name=Timezone Definitions for Mozilla Calendar
> +extensions.calendar-timezones@mozilla.org.description=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 :)
Attachment #8572914 - Flags: review?(philipp) → review+

Updated

3 years ago
Whiteboard: [timezone]

Comment 6

3 years ago
Geoff, could you provide an updated patch with the requested changes?
Flags: needinfo?(geoff)
(Assignee)

Comment 7

3 years ago
Created attachment 8659219 [details] [diff] [review]
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.
Attachment #8572914 - Attachment is obsolete: true
Flags: needinfo?(geoff)

Updated

2 years ago
Attachment #8659219 - Flags: review?(philipp)
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.
Attachment #8659219 - Flags: review?(philipp) → review+
(Assignee)

Comment 9

2 years ago
https://hg.mozilla.org/comm-central/rev/f102a2d706b3
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.7
Target Milestone: 4.7 → 4.8
You need to log in before you can comment on or make changes to this bug.