Re: [PATCH 1/3] uiribbon: Add DLL

[Alistair Leslie-Hughes]

Hi Fabian,

This is about the series of uiribbon.
> +BOOL WINAPI DllMain(HINSTANCE hInstDLL, DWORD fdwReason, LPVOID lpv)

Since nothing is implemented in this dll, please return check for
DLL_WINE_PREATTACH. (example dlls/authz/authz.c).  In this patch
you could also add the private functions.

patch 2.
> - uiribbon.idl \

...
> interface IUIFramework : IUnknown
> +{
> -
>
> +}
> -

If your going to add a idl file, please fill in methods available for
each interface.  Please also avoid the forward declare for
IUIApplication include it in the idl as well.
> -
>
> +HRESULT UIRibbonFrameworkImpl_Create(IUnknown* pUnkOuter, LPVOID* ppObj)l;

Move this function to after
+static const IUIFrameworkVtbl IUIFramework_Vtbl =
then you wont need the forward declare.

I would have the following patches.
1. Add new dll (DLLMain and private functions)
2. Add idl to include (Including all the interfaces required)
3. Add the stubbed interface.
4. Tests? (maybe not possible at this stage).

Best Regards
 Alistair Leslie-Hughes

[PATCH 1/3] uiribbon: Add DLL

[Fabian Maurer]

Hello Alistair,

Thanks for the quick feedback.
> - Add new dll (DLLMain and private functions)

Should I add all private functions like DllGetClassObject as stub, or already 
implemented? Because the class factory logic is is a bunch of code, that's why 
I separated it from the initial commit.
> - Add idl to include (Including all the interfaces required)

Just for clarification, I should add all the needed interfaces (including 
IUIApplication) at once in the idl, but only add the IUnknown methods into 
IUIFramework_Vtbl?

I'll fix the other issues, and also a few style inconsistencies I noticed. I'm 
also working on tests, but I figured it makes sense to have a stubbed 
implementation first.

Regards,
Fabian Maurer

[PATCH 1/3] uiribbon: Add DLL

[Alistair Leslie-Hughes]

Hi Fabian,
> Should I add all private functions like DllGetClassObject as stub, or already 
> implemented? Because the class factory logic is is a bunch of code, that's why 
> I separated it from the initial commit.

Just have it return CLASS_E_CLASSNOTAVAILABLE is fine.
> Just for clarification, I should add all the needed interfaces (including 
> IUIApplication) at once in the idl,

Yes, have a single patch that adds the idl.

Here is an example of what I would expect. In this case I wanted IMFTransform
but it required a a few enum/struct's to make it compile.
http://source.winehq.org/git/wine.git/?a=commit;h=2164a4da3f472c6613119fd6af...
> but only add the IUnknown methods into IUIFramework_Vtbl?

No, when you stub the interface all the functions should be added.
Example.
http://source.winehq.org/git/wine.git/?a=commitdiff;h=06f8c263c2bda30f1b08f9...

Best Regards
 Alistair Leslie-Hughes

[PATCH 1/3] uiribbon: Add DLL

[Fabian Maurer]

Hi Alistair,

thanks for the reply, I'll submit an updated patchset soon.

Regards,
Fabian Maurer