Thank you for the review.
On Jul 15, 2011, at 8:34 AM, William Schumann wrote:
Kristina,
__init__.py
OK
common.py:
OK
conv.py:
479: duplice -> duplicate
Fixed
805: is the boolean sense reversed?
Missing a not. Updated tests to reflect that condition not being caught
925: more readable, order of evaluation more obvious: not size in -> size not in
Fixed
1116: present -> presence
Fixed
1121: if device is ever None, startswith is not a method for NoneType and will
throw exception
The device should never been None but added if not None
1141: not sure what purpose of the period is here, but it appears to be dropped
on first glance
Jumpstart profiles may have code that looks like
filesys rootdisk.s0 4000 /
That code simply replaces the "rootdisk." with the actual rootdisk device so
this gets translated to
filesys c0t0d0s0 4000 /
I added an example to the comments to help make this clearer what's going on
1274: comment repeated
Removed
1279: preceeded->preceded (proceed/proceeded, precede/preceded)
1281: created like->created, such as (good message, though)
Changed to
self.logger.error(_("%(file)s: line %(lineno)d: swap "
"mount is only supported when preceded "
"by a entry that causes the root pool "
"to be created. For example root_device, "
"boot_device, pool, or "
"filesys with a mount point of '/'") % \
{"file": self.profile_name, \
"lineno": line_num})
1547: device1 was removed
Fixed
1568: "any" -> DEVICE_ANY
s/outputed/output globally
Fixed. That was the only none comment one
1890, 1899, 1908, 1910, elsewhere: why are blank lines before the end of """
comments?
When my 1st review was done for this project, Keith commented that I need to
follow this format.
It had something to do with the way emacs handles comments. Hence all my
comments follow
this pattern now.
1887, 1896: use 'pop' instead of 'fetch', if item is removed from stack
Fixed. Name of routines changed to pop_xxx
2326: code has been commented
Removed
2332: precedence (not yours)
In several instances, if 'len(x) == 0' has been replaced with 'if not len(x)'. 'not len(x)' mixes types inappropriately, since
len() returns an integer, not a boolean. Why were these changed?
2399 remove backslash
Fixed
tests look OK
William
On 07/13/11 09:33 PM, Kristina Tripp wrote:
Hi,
Could I please get a code review for the following js2ai changes
7062927 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7062927>
js2ai need to delete slices prior to create when user specified whole disk
7063455 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7063455>
js2ai add warning error if hostname and/or root password is not specified in sysidcfg
7066575 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7066575>
filesys update support for swap
7066626 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7066626>
Expand support for usedisk
7066637 <http://bugs.opensolaris.org/bugdatabase/view_bug.do?bug_id=7066637>
js2ai: error: local variable 'processed_data' referenced before assignment.
https://cr.opensolaris.org/action/browse/caiman/enpointe/CR_7062927/webrev/
These changes are primarily to address issues discovered when doing live
testing last week.
As part of filesys swap support work the main __convert_filesys_entry routine was broken out into different routines for
handling swap and / filesys entries. This does result in some duplicate logic __convert_filesys_mirror_entry and
__convert_filesys_entry But the logic is now much easier to follow and maintain now.
*Kristina Tripp, Senior Software Engineer*
Oracle Revenue Product Engineering
500 Eldorado Blvd, MS UBRM05-171
Broomfield, CO, 80021
Office: 303-272-8655
Email: [email protected] <mailto:[email protected]>
Oracle is committed to developing practices and products that help protect the
environment
_______________________________________________
caiman-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
<http://www.oracle.com/> <http://www.oracle.com/>
*Kristina Tripp, Senior Software Engineer*
Oracle Revenue Product Engineering
500 Eldorado Blvd, MS UBRM05-171
Broomfield, CO, 80021
Office: 303-272-8655
Email: [email protected] <mailto:[email protected]>
Oracle is committed to developing practices and products that help protect the
environment
