Tim,

libbe.c - 781 - needs update.

Another thing that just occurred to me is that there are many
places in beadm.py that parse a beName as a commandline argument.
Is there any particular reason why you only added the beName verify
check to those two places?


Other than that it looks fine now.


thanks,
-ethan


Tim Knitter wrote:
> 
> Ethan Quach wrote:
>>
>> Tim Knitter wrote:
>>>  
>>>> 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.
>> The message isn't correct anyway.  A BE name cannot contain many other
>> characters that you don't have listed there.  A BE name may consist of
>> alphanumeric characters, plus any of [-_.:]
>>
>> Without better error information from the library, I think this message
>> would be better left generic for now --getting rid of the second
>> sentence altogether, or summarizing the above.
>>
> 
> OK. I got rid of the second sentence.
> 
>>>  
>>>> 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.
>> Returning an int is fine.  I just want us consider what we're
>> returning here since this interface is being consumed by others,
>> and we don't have an api defined.  So right now you've got the
>> function returning 0 == Success, 6007 == Failure  Why not just
>> return 0 and 1 ?
> 
> Yeah I thought about that too and came to the conclusion that as long as the 
> success case was zero the readability of using a a defined enum outweighed 
> using 0 or 1. However 0 and 1 is clearer for consumers so I changed it to 
> that.
>  
>> libbe.c - 795 - Its returning a string instead of an int here.
>> Isn't that going to be a pain for consumers of this function?
>> Why not just return the failure int code here instead?
>>
> 
> fixed.
> 
> WR updated.
> 
> Thanks
> Tim
> 
>>
>> thanks,
>> -ethan
> _______________________________________________
> caiman-discuss mailing list
> caiman-discuss at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/caiman-discuss


Reply via email to