373436 - [he] CVS commit approval request for shipping Hebrew Thunderbird 2.0

Status: RESOLVED FIXED

(Mozilla Localizations :: he / Hebrew, defect)

Description

[Tomer Cohen :tomer]

We have bunch of changes to commit to the CVS server before Thunderbird 2.0 release, but late the freeze date. We need help approving the changes for the CVS commit. 

The diff attached fixes all the tinderbox warnings, one major XUL error (The import wizard hangs), and typos.

Comment 1

[Tomer Cohen :tomer]

Attachment: Bunch of changes for he-TB2

Comment 2

[Axel Hecht [:Pike]]

May I ask how intend to QA this patch? This affects 266 files, with many changes in toolkit, so this needs some intense testing on Firefox, too.

I think it's too late to make changes of this size for Thunderbird 2, we should try to work towards 2.0.0.1.

Can we split this up into a toolkit part and a mail part, so that we can land and test the stuff that's affecting Firefox separately?

Looking at the patch, there is a major toolchain change, too?

Comment 3

[Tomer Cohen :tomer]

Attachment: l10n/he/mail changes

The diff is a bit large due to MozillaTranslator decided to remove extra spaces from the entity files, such as the following snippet: 

+++ he/mail/chrome/branding/brand.dtd	12 Mar 2007 17:12:46 -0000
@@ -1,4 +1,4 @@
-<!ENTITY  brandShortName        "Mail/News">
-<!ENTITY  brandFullName         "Mail/News Client">
-<!ENTITY  vendorShortName       "Mail/News Client">
-<!ENTITY  sidebarName           "סרגל צד">
+<!ENTITY brandShortName "Mail/News">
+<!ENTITY brandFullName "Mail/News Client">
+<!ENTITY vendorShortName "Mail/News Client">
+<!ENTITY sidebarName "סרגל צד">

Comment 4

[Tomer Cohen :tomer]

Attachment: l10n/he/toolkit

The same goes here, most of the changes are very minor.

Comment 5

[Axel Hecht [:Pike]]

Comment on attachment 258320 [details] [diff] [review]
l10n/he/toolkit

The toolkit patch is just horrible. It's full of things that don't matter at all, like license headers, it randomly reorders stuff. I can't really do any kind of risk management for this one.

Comment 6

[Tomer Cohen :tomer]

I'll go into it in the next few days, and will try to cut the changes of toolkit into fewer files. Meanwhile, what's about the other patch?

Comment 7

[Tsahi Asher]

Axel, assuming you are not checking the correctness of the translation, what kind of risk management do you do? i know we came in too late, but now we can't ship with 2.0.
please contact me (by email or moznet: tsahi) about this.

Comment 8

[Axel Hecht [:Pike]]

sorry, dropped to follow up on this.

Any change has risk, there can be encoding problems, syntax problems, or just bad translations or anything.

By the time this patch was intended to land, it was way too short to actually intensively test the resulting build, let alone builds that would have covered regressions from this patch. At least by the timeline we had at the time. Thus, the risk of landing this patch at the time was too high, from a merely technical point.

Comment 9

[Tomer Cohen :tomer]

Now that we are after Tb2.0.0.0 release, can we consider the commit for the upcoming 2.0.0.x release? I've tested it on my local machine and it looks not harmful. 

Comment 10

[Tomer Cohen :tomer]

Following Axel advice, I'm asking for blocking 1.8.1.5. Better if it can be landed in Tb2.0.0.4. 

Comment 11

[Tomer Cohen :tomer]

Tb2.0.0.4 would be out soon (???), and the nightly are for 2.0.0.5 release already. We need it checked-in as soon as possible.

Comment 12

[Tomer Cohen :tomer]

Axel, we need your help with this. Thanks.

Comment 13

[Axel Hecht [:Pike]]

Assigning this bug to me doesn't do any good. We haven't opened the door for .5 anywhere, and once we do, I'll go through the approval requests.

This bug is on my radar well enough, reassigning back.

Comment 14

[Daniel Veditz [:dveditz]]

We have opened the door to 1.8.1.5 checkins, and code freeze is mid-July.

Axel: If this is 1.8.1.5 material please approve it relatively soon.

Comment 15

[Axel Hecht [:Pike]]

Attachment: stripped down patch

I wrote a small tool to actually ignore ordering, and then the toolkit patch strips down to 6 changes. Duh. Attaching pseudo diff for reference.
I had to make the tool ignore some line endings, too.

Comment 16

[Axel Hecht [:Pike]]

Attachment: stripped down patch for mail

Comment 17

[Tomer Cohen :tomer]

I think you should publish your tool somewhere. 

Can we commit those changes back to the CVS?

Comment 18

[Axel Hecht [:Pike]]

The diffs I attached aren't real diffs, as I use a properties-like markup even for DTDs. Anyway, I'll make a judgement on the original patches tomorrow.

Comment 19

[Axel Hecht [:Pike]]

Comment on attachment 258317 [details] [diff] [review]
l10n/he/mail changes 

[he] approval for toolkit patch. Please test the results intensively on both FX and TB, even though the actual changes should be small, according to my tools.

Nit: 

-# #define MOZ_LANGPACK_CONTRIBUTORS <em:contributor>Reuven Gonen</em:contributor> <em:contributor>Tsahi Asher</em:contributor>
+# #define MOZ_LANGPACK_CONTRIBUTORS <em:contributor>Reuven Gonen</em:contributor> <em:contributor>Tsahi Asher</em:contributor> <em:contributor>Tomer Cohen</em:contributor>

won't have any effect until you remove the leading "# ", resulting in a 
#define MOZ_...

As that can break the language pack, let's make that in a follow up patch.

Comment 20

[Axel Hecht [:Pike]]

Comment on attachment 258320 [details] [diff] [review]
l10n/he/toolkit

[he] mail patch approved for tb2+. Or vice versa, pff.

Please use the fixed1.8.1.5 and verified1.8.1.5 keywords to track your landing and testing.

Comment 21

[Jeferson Hultmann]

Tomer, I wrote a tool to sanitize Mozilla Translator exported files. I think it would be useful for this bug. :)
http://groups.google.com/group/mozilla.dev.l10n/browse_thread/thread/e623f7dff499975b/8757918f5301b59b

command line:
sync.exe cvs/mozilla1.8/source/toolkit/locales/en-US mt-export/toolkit cvs/mozilla1.8-l10n/he/toolkit

1. cvs/mozilla1.8/source/toolkit/locales/en-US: mozilla source from CVS
2. mt-export/toolkit: MT, Export > CVS Translation
3. cvs/mozilla1.8-l10n/he/toolkit: files created by sync.exe (commit these files to CVS)

Comment 22

[Daniel Veditz [:dveditz]]

Comment on attachment 258317 [details] [diff] [review]
l10n/he/mail changes 

Too late to land in 1.8.1.5, gone into firedrill mode. Try next time.

Comment 23

[Tomer Cohen :tomer]

We did landed it to the CVS about a week ago. Can you consider it again?

Comment 24

[Axel Hecht [:Pike]]

Comment on attachment 258317 [details] [diff] [review]
l10n/he/mail changes 

Yep, this landed.

Tomer, could you please set the keywords to fixed1.8.1.5 or verified1.8.15 to indicate whether this just landed or if you actually tested the resulting builds yet?

If you haven't tested yet, please do.

Comment 25

[Axel Hecht [:Pike]]

Actually, I'm gonna set fixed1.8.1.5 right away to make dveditz' queries happy.

Comment 26

[Tomer Cohen :tomer]

(In reply to comment #24)
> If you haven't tested yet, please do.

We do testing it, so far we got non of any blocking bugs. 

Tinderbox is still not green, and I guess I should open separated bug for that.