[pve-devel] Proxmox VE 2.0 beta 4 i18n patch

Dietmar Maurer dietmar at proxmox.com
Wed Dec 7 09:40:30 CET 2011


Hi Koichi,

you patch is simply too large. I always tell people to send small patches, so that I can review code and add my comments.

Your patch is more than 8000 Lines, so it would take a week or more to review the whole patch.

Anyway, here is the review of the changes you made to Utils.js:

1.)  I replaced the string 'Other' with 'Other OS types'

2.) You can't use gettext in performance critical render functions:

     render_kvm_ostype: function (value) {
 	if (!value) {
-	    return 'Other';
+	    return gettext('Other');
 	}

This is a performance killer. Instead I usually use a property which is initialized once
to store the translated text:

    defaultText: gettext('Default'),

and the use that in performance critical functions:

    return PVE.Utils.defaultText;

3.) I am not sure if it really make sense to translate product names like 'Microsoft Windows 2000' or 'Cirrus Logic GD5446'. Do you think that is really needed (why)?

4.) I would not translate language names like gettext('Japanese'). We only use those texts in the language selector, and it is probably better to always display the English version.

5.) You changed some already translated strings like:

    var msg = gettext('Connection error') + ' - server offline?';

to 
    var msg = gettext('Connection error') + ' - ' + gettext('server offline?');

You should first clarify why it is done that way. The reason is that error messages from the server are not translated anyways.
So for now I would leave that as it is.

6.) It is sometimes a good idea to change texts to minimize translation overhead. For example:

-	qmcreate: 'Create VM {0}',
+	qmcreate: gettext('Create VM {0}'),
-	vzcreate: 'Create CT {0}',
+	vzcreate: gettext('Create CT {0}'),

I would use the following instead (split action from object):

+	qmcreate: gettext('Create {0}'),
+	vzcreate: gettext('Create {0}'),	

Sure, that needs further changes in the code. I will commit those changes soon. Please can you
verify that this works for Japanese too (you need to change those texts in ja.po)
 
I will try to include your changes in small steps. Please be patient - this will take some time.

- Dietmar

> -----Original Message-----
> From: pve-devel-bounces at pve.proxmox.com [mailto:pve-devel-
> bounces at pve.proxmox.com] On Behalf Of Koichi MATSUMOTO
> Sent: Dienstag, 06. Dezember 2011 12:16
> To: pve-devel at pve.proxmox.com
> Subject: Re: [pve-devel] Proxmox VE 2.0 beta 4 i18n patch
> 
> Hi,
> 
> I have posted a i18n patch and Japanese translation. Please take a look.
> 
> Subject: [PATCH 1/2] Modifications for i18n
> Subject: [PATCH 2/2] Ja translation



More information about the pve-devel mailing list