On Wed, Oct 22, 2008 at 11:33:52AM +0100, jmr wrote:
> http://cr.opensolaris.org/~jmr/um_4126_v1_Oct22/ips_gate.patch
Makefile:
- Why do you cd into po? I don't see a po directory, either in the
current gate or created in your patch. Should you be moving gui/po to
po?
gui/Makefile:
- line 171: you can use $(<D) instead of the whole $DIR thing.
beadmin.py:
- line 434: why all the encoding support? Will date_format ever be
non-ASCII? You don't catch UnicodeError here -- couldn't that be a
problem when calling encode()? Why is all the encoding stuff done here
and nowhere else?
image.py:
- line 493: remove this comment
- line 495: Why is this the only place in all the code you make this
change? There are two other places in this file alone that have the
same problem, and I'm sure there are tons of other places in other
files.
misc.py:
- line 381: You shouldn't need this -- gettext.install() need only be
called once for the process, so once it's in client.py and
packagemanager.py, you're done. In general, your code does a bunch of
different things to try to get at the localized text. In each
program's main() or equivalent, you should call gettext.install() with
the appropriate arguments, and use _() everywhere else. But some
places you use bindtextdomain() and textdomain() either in addition to
or in place of gettext.install(), and you add a "_" member to objects
pointing to gettext.gettext() rather than just using the builtin _()
function, and in some places you create a N_() method ... it's all over
the place.
- line 384: Are these symbols really translated? They're SI standard.
packagemanager.py:
- line 99 et al: you don't need continuation characters inside of parens
(or brackets or braces); please remove them in lines you're changing,
and clean them up as you encounter them in future fixes.
updatemanagernotifier.py:
- I don't understand this change. Why is it necessary (other than the
/usr/lib to /usr/share change)? Why is it not consistent across all
the programs?
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
