394633 - Source L10n for venkman (work with CVS localization)

Status: RESOLVED FIXED

(Other Applications Graveyard :: Venkman JS Debugger, defect)

Description

[Robert Kaiser]

After doing this in bug 394315 for ChatZilla, I think getting source L10n in place for venkman should be very similar :)

Comment 1

[Robert Kaiser]

Attachment: cvs copies needed for venkman source L10n

Here goes the same game as for ChatZilla. ;-)

First part is once again the CVS copies to the new locales/ directory.

Comment 2

[:Gijs (he/him)]

Comment on attachment 279322 [details]
cvs copies needed for venkman source L10n

Been there, seen that - didn't get a t-shirt.

r=gijs, assuming we get the makefile and makexpi.sh stuff next. :-)

Comment 3

[Robert Kaiser]

Attachment: patch for enabling source L10n

Here's the actual patch. Basically that does the same as the ChatZilla patches in bug 394315 - only that all-locales is empty atm and I'm also fixing a wrong pointer to CONFIGDIR in makexpi.sh - and I'm removing the ifdef MOZ_XUL_APP in the Makefiles, as all users of those Makefiles are MOZU_XUL_APPs anyways.

Comment 4

[:Gijs (he/him)]

Comment on attachment 279325 [details] [diff] [review]
patch for enabling source L10n

r=gijs, with the same note about makefiles as I gave in the other bug, so it might be prudent to get luser or bsmedberg to check your changes here, too

Comment 5

[Robert Kaiser]

Comment on attachment 279325 [details] [diff] [review]
patch for enabling source L10n

luser, could you take a look at the Makefile.in changes here? This should be basically the same as you have seen in the ChatZilla example I pointed to on IRC, only that venkman already has logic for registering as a toolkit-style extension.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 279325 [details] [diff] [review]
patch for enabling source L10n

Index: mozilla/extensions/venkman/locales/Makefile.in
===================================================================
RCS file: mozilla/extensions/venkman/locales/Makefile.in
...
+# include config.mk before we override the AB_CD var it sets
+include $(topsrcdir)/config/config.mk
+
+# if the wanted language is not in all-locales, fall back to en-US
+ifeq (,$(filter $(AB_CD),$(shell cat $(srcdir)/all-locales)))
+AB_CD = en-US
+endif
+
+DEFINES += -DAB_CD=$(AB_CD)
+
+VENKMAN_VERSION=$(shell cat $(srcdir)/../version.txt)
+
+XPI_NAME = venkman
+NO_JAR_AUTO_REG = 1
+INSTALL_EXTENSION_ID = {f13b157f-b174-47e7-a34d-4815ddfdfeb8}
+XPI_PKGNAME = venkman-$(VENKMAN_VERSION)
+

You need to move the lines from VENKMAN_VERSION to XPI_PKGNAME before you include config.mk (see suite/debugQA/locales/Makefile.in for an example). Otherwise venkman will end up with an installed-chrome.txt (in its chrome directory) which it doesn't need.

Comment 7

[Axel Hecht [:Pike]]

We shouldn't do source l10n in the l10n repository without a concrete contract between the module owner and localizers and what is actually expected to happen.

Not sure who's actually owning venkman right now in the first place, so this seems to be the wrong thing to do right now.

Comment 8

[Dave Miller [:justdave]]

Since it wasn't mentioned elsewhere on this bug yet (other than a mysterious dependency listing ;) ), the CVS copies that go with this are on bug 394638.

Comment 9

[Robert Kaiser]

In bug 397246, we now have resolved the story for ChatZilla in a way that works for everyone, and I'm planning to do the same for venkman now.
The solution is to do CVS-based dependent language packs, which leave the main extension package to be the same everywhere, with optional language packs that have extension dependencies and are pre-installed in localized SeaMonkey when the extension itself is installed.

Comment 10

[Robert Kaiser]

Comment on attachment 279325 [details] [diff] [review]
patch for enabling source L10n

obsoleting this patch, I'll work on a new one for the new solution.

Comment 11

[Robert Kaiser]

Attachment: patch v2: use CVS-based language packs

Here's the new patch. I also have done some cosmetic changes to makexpi.sh to sync it with ChatZilla's version, big parts of the patch are just "stolen" from what ChatZilla does now and adapted for venkman.

Comment 12

[:Gijs (he/him)]

Comment on attachment 295365 [details] [diff] [review]
patch v2: use CVS-based language packs

This looks good from what I can tell - I'll assume the copy-paste work was done correctly. :-)

However, my review is not good enough for the suite portions, you would theoretically need to ask someone else for that rubberstamp, but that's for you to decide.

Finally, I just realized this probably means we should CVS rm the locale-resources stuff in ChatZilla as well as remove/alter the tutorial we have on creating a langpack, right?

Comment 13

[Robert Kaiser]

BTW, Gijs has agreed on IRC to extend his review to adding extensions/venkman/locales/Makefile to extensions/venkman/makefiles.sh, which I forgot in the patch, but realized when doing a clobber.

Comment 14

[Robert Kaiser]

I checked this in, some tinderboxen did apparently have a problem finding the new locales Makefile, but re-running configure, which should have been triggered by a different checkin roughly at the same time, should fix that.

Comment 15

[Robert Kaiser]

As everything went well with the checkin, I now have cvs removed the now obsolete files in resources/locale/ - this bug should be fixed now.