Hi Mary,

I corrected Cc line so please use this one when you reply.

Thanks,
Takeshi

On 2012年04月03日 14:44, Takeshi Asano wrote:
Hi Mary,

Thanks for the comments.

On 2012年04月03日 06:11, Mary Ding wrote:
Jack and Pavel:

Please make sure these are pep8 and pylint clean.

I tried pep8 in the way described in usr/src/README, i.e.:
/usr/bin/pep8 --ignore=E261,W291,W293 --count -r --show-source usr
and got many E501 (line too long) and other types of errors/warnings.

- among files got E501 (line too long), these files are which touched
by the webrev so Pavel please check them and fold if lines we touched
are exceeding 80 chars.
usr/src/cmd/gui-install/src/__init__.py
usr/src/cmd/installadm/installadm_common.py
usr/src/cmd/js2ai/modules/common.py
usr/src/cmd/system-config/__init__.py
usr/src/cmd/text-install/__init__.py
usr/src/lib/install_manifest_input/__init__.py
Other files are not be touched by the webrev.

- none of files got other types of errors/warnings are touched by the webrev.

Regarding pylint, is there any configuration file for other standard
way for slim gate?

Since changes in .py files by the webrev are message domain name updates
and fold of lines, value to apply pylint is to detect unintended misedit.
But simply applying pylint to each of the .py files touched looks to
generate other errors/warnings.

I will also like to know what kind of testing had been done with these changes:

1. Have install_unit_test had been run ???

No,although this time's change should not affect functionality of
install components, running the suite will be good to detect
regression. I'll try that.
If you have additional information to usr/src/tools/tests/REDAME,
please let me know.

2. Since this is related to globalization, have there been any install testing 
being done with a particular locale ???

- Newly generate .po files as part of slim gate build
-> confirmed that don't see unexpected changes in generated
files, compared with older localizable files G11N generated
from slim gate.

- Message domain name changes and move to /usr/share/locale
-> Since currently localized files are integrated by G11N
as system/install/locale, testing for this is covered
by G11N's PIT for the package.

- Locale-sensitive installation (facet.locale handling)
-> this time's change does not touch/affect this at all

Thanks,
Takeshi







----- Original Message -----
From: [email protected]
To: [email protected]
Cc: [email protected], [email protected]
Sent: Monday, April 2, 2012 1:50:51 PM GMT -08:00 US/Canada Pacific
Subject: Re: [caiman-discuss] Please review: processing changes to 
globalization data

Hi,

I have a few comments:

- General comment: Please make sure all the lines obey the 80 characters per 
line limit.
I noticed a few files in the "20120326-pheimlic-installer-IPS0-src" list of 
files that have lines
longer than 80 characters.

usr/src/cmd/js2ai/modules/conv_sysidcfg.py:

line 1319: Question: is it really true that this path for S10 is also changing?
Just want to make sure this is an intended change, and not a change because
of global search and replace?

usr/src/Makefile.master:

line 104: Adding the use of /usr/gnu/bin/grep makes the buiding of the
slim_source gate dependent on the pkg:/text/gnu-grep package. Do you know
whether this package is installed by default? If not, please update
the usr/src/README to document that the gnu-grep package is required to
build slim_source.

usr/src/cmd/system-config/Makefile:

line 59: Why is profile/user_info.py listed here, while other files in
the profile directory is not? I think it would be useful to have a comment
here to explain.

Thanks,

--Karen

On 04/02/12 11:41, Jack Schwartz wrote:

Hi everyone.

I am posting this code review for Takeshi Asano and Pavel Heimlich of the 
Globalization Team since they were having issues posting directly to 
cr.opensolaris.org. They are making changes to our gate to create po files 
(libraries of messages to localize) and deliver them in their own package. The 
webrev is split into two: one for Makefiles, the other for source files.

They are targeting build 14, so please review by Wednesday 4/4 COB.

https://cr.opensolaris.org/action/browse/caiman/schwartz/7117291_1

Be sure to include their email addresses on any replies.

Thanks,
Jack


_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss




_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss

Reply via email to