Sarah Jelinek wrote:
>
>
> Joseph J. VLcek wrote:
>> Thanks for the feedback Sarah. My comments below.
>>
>>
>>
>> Sarah Jelinek wrote:
>>> Hi Glenn,
>>>
>>> Apologies for the delay in getting back to you on this. A couple of
>>> nits:
>>> 1. The comment in the webrev should be the bug id, synopsis before
>>> putback
>>>
>>> 2. vmc_common:
>>> -Why are the vmc_error_handler_trap() and vmc_error_handler() not
>>> prefixed with the 'function' qualifier? They are used in other
>>> scripts, right? Maybe I misunderstand the use of 'function' in ksh93.
>> I commented this issue in the function headers:
>>
>> 150 # This function is not defined using the function keyword
>> 151 # to avoid an exit loop.
>>
>> Did you miss that comment or is more description needed?
Hi Joe,
Yes, apparently I did miss this comment :-). Sorry for the churn. Thank
you for pointing this out.
sarah
*****
>
> Hi Joe,
>
> It isn't the description it's that we don't have this method defined as
> a 'function'. Which may be fine, I am just wondering what the difference
> is.
>
> For example, in this file you have:
>> 131 function vmc_set_new_iso_path
>> 132 {
>
> But, for this method there is no 'function' qualifier in the declaration:
>> 164 vmc_error_handler_trap ()
>> 165 {
>
> Just wondering what the difference is between these methods.
>
> Also, I just noticed the comment block above the vmc_set_new_iso_path is
> incorrect:
>> 118
>> #######################################################################
>> 119 #
>> 120 # vmc_is_vbox_installed
>> 121 #
>
> Should be:
>
> # vmc_set_new_iso_path.
>
> thanks,
> sarah
> ***
>
>
>
>
>
>
>
>>
>> Joe
>>
>>>
>>>
>>> that's all I have.
>>>
>>> thanks,
>>> sarah
>>> ****
>>> Glenn Lagasse wrote:
>>>> The following link will display the updated webrev for the VMC project.
>>>> It is a diff against what we submitted initially.
>>>>
>>>> <http://cr.opensolaris.org/~glagasse/webrev.diffs>
>>>>
>>>> We would like to get feedback on this round of changes no later than
>>>> COB
>>>> this Friday if at all possible. If you would like to review these
>>>> changes but can't be done with your comments by then please ping me and
>>>> we'll work something out.which
>>>> CAVEATS:
>>>>
>>>> o All pep8/pylint comments not addressed in this webrev for anything
>>>> other than im_pop.py will be addressed when we merge with the python2.6
>>>> work and pick up Jean's fixes.
>>>>
>>>> o Some of the diffs will show changes that occurred in slim_source
>>>> outside of VMC. This is the unfortunate result of my not reusing my
>>>> initial workspace to address the code review comments. I could go back
>>>> and do this if it's a real pain point to people (but would prefer
>>>> not to
>>>> at this point).
>>>>
>>>> o There's a small change coming for prepare_ai_image due to the
>>>> bootroot/boot_archive changes in slim_source. That will be handled
>>>> before integration and sent out as a small seperate review (essentially
>>>> a 4-line change).
>>>>
>>>> o .image_info file creation. One of the review comments I received was
>>>> that .image_info creation wasn't optimal. If the file can't be created,
>>>> the DC build continues on even though the resulting media that needs
>>>> the
>>>> .image_info file (liveCD at the moment) won't work. I was going to
>>>> clean this up in a generic way such that only media types that require
>>>> it actually generate it. I wasn't able to come up with a solution that
>>>> I liked in the time I allotted. Since VMC isn't regressing anything in
>>>> regards to the .image_info file I'm instead going to file a bug against
>>>> DC and fix this post-putback outside of VMC.
>>>>
>>>> Thanks,
>>>>
>>>> Team VMC (Joe and Glenn)
>>>> _______________________________________________
>>>> caiman-discuss mailing list
>>>> caiman-discuss at opensolaris.org
>>>> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
>>
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss
