Swati/Mary,
Technically Mary is correct. You should alphabetize your imports. I
personally take it a step further, as outlined in the pep8 guide -
http://www.python.org/dev/peps/pep-0008/
Imports are always put at the top of the file, just after any module
comments and docstrings, and before module globals and constants.
Imports should be grouped in the following order:
1. standard library imports
2. related third party imports
3. local application/library specific imports
You should put a blank line between each group of imports.
In your case, I would move the import (in
text-install/disk_selection.py) before line 37.
import <built-ins>
import <local application imports>
from math import ceil <----
from bootmgt.bootinfo import .....
<rest of "from" imports>
And something similar in the GUI code.
Otherwise, LGTM.
-Drew
On 6/27/12 3:37 PM, Mary Ding wrote:
Swati:
1. You should alphabetize your import, so
from math import ceil
should be higher up in line 40
2. For your tests with livecd and text install, did you really go and
do an install with the Minimum and Recommended size and make sure they
work.
On 06/27/12 02:18 PM, Swati Sarraf wrote:
Hi All,
Can I have a code review for the following bug fix:
7163694 Interactive installer's display of min/rec install device
size display should be rounded up
http://monaco.us.oracle.com/detail.jsf?cr=7163694
Webrev link:
https://cr.opensolaris.org/action/browse/caiman/ssarraf/7163694/
Testing is done as follow:
1. slim test done,result OK
test-pointer: /net/osol-bldx/datapool/ssarraf/7163694/slimtest-27june
2. pep8 and pylint test done, result OK
pylint-pointer: /net/osol-bldx/datapool/ssarraf/7163694/pylint-7163694
pep8-pointer : /net/osol-bldx/datapool/ssarraf/7163694/pep8-7163694
3. Created DC image for text installer and gui-livecd. To test the
changes I printed the original value and roundup value in install log
for both. looked fine.
For text-installer image, install log showed:
Text_rec_orig: 4.7240610
Text_rec_roundup: 4.8
Text_min_orig: 2.7240610
Text_min_roundup: 2.8
For gui-livecd image, install log showed:
Gui_rec_orig: 7.0
Gui_rec_roundup: 7.0
Gui_min_orig: 5.0
Gui_min_roundup: 5.0
I also took snapshot of text installer and gui livecd size screen>
test installer pointer:
/net/osol-bldx/datapool/ssarraf/7163694/text-screen.png
gui livecd pointer: /net/osol-bldx/datapool/ssarraf/7163694/gui.png
-Thanks
Swati Sarraf
_______________________________________________
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
