oledlg: Implement OleUIAddVerbMenuA/W functions [resend]

Nikolay Sivov bunglehead at gmail.com
Thu Dec 25 11:35:05 CST 2008


Vitaly Lipatov wrote:
> В сообщении от 25 декабря 2008 Вы написали:
> ...
>   
>> You could start with testing for some special cases (such as NULL input
>> parameters).
>>     
> If I will add test case for NULL input and returned values, my commit will be 
> accepted?
>   
I don't know actually. That's first I think about when my patch isn't 
accepted.
>> For me your comment looks suspicious:
>>     -----
>>
>> 	+ * TODO
>> 	+ *  Check if OLE object has no verbs
>>     
>
>
>   
>>     -----
>>
>> 	+    hr = IOleObject_EnumVerbs(lpOleObj, &pEnumVerbs);
>> 	+    if (hr == S_OK) {
>>   ----
>> Will this hr be OLEOBJ_E_NOVERBS when no verbs present?
>> If it's so you're actually checking for this.
>> This one needs a testcase for example.
>>     
> Please understand me. I did a part of work. Possible my implementation is 
> incomplete. But I wish to share result of my work with the world, winehq 
> community. I agree with test case argument with I fix already existing 
> function. But I do a new realization and have no enough time and knowledge
> to write ideal code. Also I can improve code only from winehq repository.
> ...
>   
All of this is perfectly clear, and I don't argue with you and I'm not 
calling your patch bad.
I'm only point on some obvious things for me. Treat this only like an 
advice and my opinion.
>> Does native crash on it or not? I'm not sure.
>>
>> The same about SetLastError on failure - this should be checked by testing
>> it directly.
>>     
> I will try to add this tests.
>   



More information about the wine-devel mailing list