Accidentally sent that out before answering one last question you had
>>
>> 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?
It was review comment on my last review. I've flipped them all back to len(x).
I'll update the webrev once I'm completed retesting the changes
On Jul 15, 2011, at 9:19 AM, Kristina Tripp wrote:
>
> 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 js2ai need to delete slices prior to create when user specified
>>> whole disk
>>> 7063455 js2ai add warning error if hostname and/or root password is not
>>> specified in sysidcfg
>>> 7066575 filesys update support for swap
>>> 7066626 Expand support for usedisk
>>> 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]
>>>
>>> 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
>
>
> 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]
>
> Oracle is committed to developing practices and products that help protect
> the environment
>
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]
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
