Ethan,

> Tim,
> 
> Just a couple more comments...
> 
> BootEnvironment.py - 42 - Where is this variable used?
> 

No where. I was initially going to store the max length there but used a class 
variable instead. I removed it.
  
> beadm.py - 193,649 - why does the '%' symbol have to be
> passed in as a variable into the error message string?
> 

It wasn't possible to escape it in the message string so I had to pass it in. 
Python kept interpreting it to be a format character.
 
> libbe.c - 799 - should this somehow be a boolean
> function instead?
> 

There isn't a way to pass back a true boolean. Or put another way, there isn't 
a boolean format specifier when using Py_BuildValue(). There is the "b" 
specifier but when used it doesn't produce a Python "True" or "False". It 
returns an int so I left this as is.

Webrev updated.

Thanks
Tim

> 
> thanks,
> -ethan
> 
> 
> Tim Knitter wrote:
>>
>>
>> Ethan Quach wrote:
>>>
>>>
>>> Tim Knitter wrote:
>>>>
>>>> Hi Evan,
>>>>
>>>>> Hi Tim,
>>>>>
>>>>> I understand the reasoning behind the length limitation for the BE 
>>>>> name but I'm not clear why the description is also limited to 64 
>>>>> characters. What was the reasoning behind the description length 
>>>>> limit?
>>>>>
>>>>
>>>> None other than having a sane limit so the text doesn't wrap in grub 
>>>> and in the boot menu (menu.lst for x86).
>>>
>>> If there's no technical reason why we should be limiting this, I
>>> think we should leave it alone.  What's so magical about 64 being
>>> within limits of a wrapped-around grub title anyway?
>>>
>>
>> Nothing magical maybe a little mystical though. :-) I removed 
>> description checking throughout as well as checking the length of the 
>> root dataset before zfs works on the dataset later in the code as was 
>> discussed in the snap meeting.
>>
>> The webrev has been updated.
>>
>> Thanks
>> Tim
>>
>>>
>>> -ethan
>>>
>>>
>>>>
>>>> Thanks
>>>> Tim
>>>>
>>>>> Thanks!
>>>>> -evan
>>>>>
>>>>> Tim Knitter wrote:
>>>>>> Please review the following bug fix:
>>>>>>
>>>>>> 5749 libbe to provide public interface to validate BE name
>>>>>>
>>>>>> http://defect.opensolaris.org/bz/show_bug.cgi?id=5749
>>>>>> http://cr.opensolaris.org/~tsk/5749/
>>>>>>
>>>>>> Thanks
>>>>>> Tim
>>>>>> _______________________________________________
>>>>>> 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

Reply via email to