On 2/4/10 10:29 AM, Glenn Lagasse wrote:
> Hi Evan,
>
> * Evan Layton (Evan.Layton at Sun.COM) wrote:
>> On 2/3/10 2:11 PM, Glenn Lagasse wrote:
>>> Could I please get two reviewers to have a look at this 2010.03 stopper?
>>>
>>> 13272 Need to add support for customizing default AI client manifest on
>>> AI image for VM construction
>>>
>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=13272
>>>
>>> Webrev:
>>>
>>> http://cr.opensolaris.org/~glagasse/fixups/
>>>
>>> Thanks!
>>>
>>
>> Hi Glenn,
>>
>> I had a comment similar what Karen asked about for line 117 in
>> prepare_ai_image.
>>
>> If this is optional you could make the code match the comment.
>> Doing something like checking for 6 arguments (or more) and less
>> than 8 and then filling in AI_MANIFEST="default" if nothing is
>> passed in would be one way to provide this as an optional parameter.
>>
>> Just a thought... :-)
>>
>> Other than that the changes look fine to me.
>
> See my response to Karen.  I've updated the comment to be more in-line
> with the code and updated the webrev.
>
> Thanks for the review Evan!
>

OK, it looks fine now.

-evan

Reply via email to