Shawn Walker wrote:
> Brock Pytlik wrote:
>> New webrev which moves the logic into find_root and I hope changes 
>> the GUI appropriately.
>> http://cr.opensolaris.org/~bpytlik/ips-1651-v2/
>
> image.py:
>   line 104: docstring please
>
>   line 252: space needed after ','
>
> packagemanager.py:
>   lines 1199ff: since you changed this, can you change the ' to " for 
> the strings and s/not valid/not a valid/ ?
>
>   line 1209: s/is not None// simpler?
It might be simpler, but I believe is not None is clearer and more 
robust since that's exactly the test I wish to make. If, in the future, 
image_obj can be True or False, for example, I'd like this code to blow 
up here when it tries to do access the history member of a boolean, 
rather than after the value has escaped from this method.
>
> Otherwise, looks fine.
>
> Cheers,
Webrev updated to reflect these changes:
http://cr.opensolaris.org/~bpytlik/ips-1651-v3/

Thanks for taking a look Shawn,
Brock
_______________________________________________
pkg-discuss mailing list
[email protected]
http://mail.opensolaris.org/mailman/listinfo/pkg-discuss

Reply via email to