Re: Request for review on German localizations for icedtea-web

[Jacob Wisor]

Hello!

@Jiri:
Sorry, I did not have time to test your patch yet. I had to do some stupid stuff for the german bureaucracy (sigh). But, a week on vacation has made me forget almost all troubles. ;)

"Adam Domurad"<adomurad at redhat.com> wrote:
>> +
>> +This file is part of IcedTea.
>> +
>> +IcedTea is free software; you can redistribute it and/or
>> +modify it under the terms of the GNU General Public License as
>> published by
>> +the Free Software Foundation, version 2.
>> +
>> +IcedTea is distributed in the hope that it will be useful,
>> +but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
>> +General Public License for more details.
>> +
>> +You should have received a copy of the GNU General Public License
>> +along with IcedTea; see the file COPYING. If not, write to
>> +the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>> Boston, MA
>> +02110-1301 USA.
>> +
>> +Linking this library statically or dynamically with other modules is
>> +making a combined work based on this library. Thus, the terms and
>> +conditions of the GNU General Public License cover the whole
>> +combination.
>> +
>> +As a special exception, the copyright holders of this library give you
>> +permission to link this library with independent modules to produce an
>> +executable, regardless of the license terms of these independent
>> +modules, and to copy and distribute the resulting executable under
>> +terms of your choice, provided that you also meet, for each linked
>> +independent module, the terms and conditions of the license of that
>> +module. An independent module is a module which is not derived from
>> +or based on this library. If you modify this library, you may extend
>> +this exception to your version of the library, but you are not
>> +obligated to do so. If you do not wish to do so, delete this
>> +exception statement from your version.
>> + */
>> +package net.sourceforge.jnlp.config;
>> +
>> +import org.junit.Assert;
>> +import org.junit.Test;
>> +
>> +public class BasicValueValidatorsTests {
>> +
>> + //decomposed for testing
>> + public static boolean canBeWindows(String s) {
>> + //is just win bullet proof enough? SOme winfows mark themselves as
>> win32 or similar
>> + //but I can imagine linux distribution with some Name containing win:(
>> + //so I guess windows should be good enough as I have not seen
>> not-windows idf since XP
>> + return s.toLowerCase().contains("windows");
> 
> s/SOme/some/, s/winfows/Windows/, s/Name/name/
> idf -> ???

idf -> identifier ;) Probably...
 
> Anyway, I haven't see any reference that os.name can return 'win32', do
> you have one ?
> [nit] I'd be in favour of a shorter note here, it's a bit of a ramble :-)

Right, please use more generic and neutral language. Try to avoid 'I'.

> > + }
> > +
> > + /**
> > + * guess the OS of user, if legal, or windows
> > + * @return
> > + */
> > + public static boolean canOsBeWindows() {
> > + return canBeWindows(System.getProperty("os.name"));
> > + }
> 
> >From what I read on the internet,
> System.getProperty("os.name").startsWith("Windows") is the accepted way
> to do this. (I don't think we need to worry about this being inadequate
> until we have proof it doesn't handle some version of windows).
> [nit] I'd just call it isWindows() or isOsWindows() or similar.

Agreed, isWindows() or isOSWindows() are better fitting names. But, the documentation should probably mention that the check is not rock-solid or that it is based on the "os.name" property.
Unfortunately, there is no way in Java to check precisely for a specific OS, unless you try to load a native library and do the actual OS check in that native code. AFAIK the J2SE specification is not clear enough on what the value of the "os.name" property should look like. It is ment to be just some kind of note than a bullet proof means to determine the underlying OS. So, as long as the value is checked for a "Windows" sub-string in all of its variations, it should be enough.

Regards,

Jacob