Hi John,
Thanks for the review.
updated webrev: https://cr.opensolaris.org/action/manage/caiman?__fsk=1948518347
-Thanks
Swati Sarraf
----- Original Message -----
From: john.fisc...@oracle.com
To: swati.sar...@oracle.com
Cc: caiman-discuss@opensolaris.org
Sent: Thursday, July 19, 2012 2:21:51 PM GMT -08:00 US/Canada Pacific
Subject: Re: [caiman-discuss] Code Review request: 7183018
Swati,
Looks good. Only a nit on the LOGGER.debug() when the image
size is unavailable. Instead of using the following text:
> 655 LOGGER.debug("Unable to get image size from image_info
> file, "
> 656 "so using fallback image size")
Can you use the text that is used by the text-installer?
> LOGGER.debug("Unable to get image size")
If so then I do not need another webrev.
Thanks,
John
On 07/19/12 12:23 PM, Swati Sarraf wrote:
> Hi All,
>
> This is a stopper so it would be great if you can spare some time to review
> it.
>
>
> -Thanks
> Swati Sarraf
>
>
> ----- Original Message -----
> From: susan.s...@oracle.com
> To: swati.sar...@oracle.com
> Cc: caiman-discuss@opensolaris.org
> Sent: Tuesday, July 17, 2012 2:41:29 PM GMT -08:00 US/Canada Pacific
> Subject: Re: [caiman-discuss] Code Review request: 7183018
>
> On 07/17/12 02:12 PM, Swati Sarraf wrote:
>> Hi,
>>
>> Thanks for the review. please find the updated webrev:
>> https://cr.opensolaris.org/action/browse/caiman/ssarraf/7183018/
>> Regarding the indentation of line 655: I saw two different indentation
>> method in line 679 and line 665. which one is correct?
> 679 is better in this instance, since you have space on the line.
>
> Sue
>
>> ----- Original Message -----
>> From: susan.s...@oracle.com
>> To: swati.sar...@oracle.com
>> Cc: caiman-discuss@opensolaris.org
>> Sent: Tuesday, July 17, 2012 1:45:06 PM GMT -08:00 US/Canada Pacific
>> Subject: Re: [caiman-discuss] Code Review request: 7183018
>>
>> On 07/17/12 11:47 AM, Swati Sarraf wrote:
>>> Hi All,
>>>
>>> Can I have a code review for the following bug fix:
>>>
>>> 7183018 Gui livecd is not using size from image.info but using fallback
>>> image size
>>> http://monaco.us.oracle.com/detail.jsf?cr=7183018
>>>
>>> Webrev link:
>>> https://cr.opensolaris.org/action/browse/caiman/ssarraf/7183018/
>>>
>>> Testing is done as follow:
>>>
>>> 1. Slim test done
>>> Ran 1901 tests in 353.236s, FAILED (SKIP=1, failures=1)
>>> failed test: Tests package with understood add option
>>> Slim test pointer: file:///net/osol-bldx/datapool/ssarraf/7183018/
>>>
>>> 2. Pylint and pep8 test done , result OK
>>> pylint result pointer: file:///net/osol-bldx/datapool/ssarraf/7183018/
>>>
>>> 3 Created DC image and tried installation with minimum size (4.7GB).
>>> Installation finished successfully.
>>>
>>>
>>> -Thanks
>>> Swati Sarraf
>>> _______________________________________________
>>> caiman-discuss mailing list
>>> caiman-discuss@opensolaris.org
>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>> Hi Swati,
>>
>> I had all the same comments as Karen, plus a few more nits:
>>
>> 651 No need for backslash.
>>
>> 652 Should be indented so that "Size.mb_units is lined up with str on line
>> above
>>
>> 654
>> if its fail
>> ->
>> if it fails
>>
>> Sue
>> _______________________________________________
>> caiman-discuss mailing list
>> caiman-discuss@opensolaris.org
>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss@opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss@opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
_______________________________________________
caiman-discuss mailing list
caiman-discuss@opensolaris.org
http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
