Re: [PATCH 2/8] d3drm: Specify when a method is forwarded to another interface. (resend)

[Alexandre Julliard]

Christian Costa  writes:
> @@ -438,7 +438,7 @@ static HRESULT WINAPI IDirect3DRMMeshBuilder2Impl_Load(IDirect3DRMMeshBuilder2*
>  {
>      IDirect3DRMMeshBuilderImpl *This = impl_from_IDirect3DRMMeshBuilder2(iface);
> - TRACE("(%p)->(%p,%p,%x,%p,%p): partial stub\n", This, filename, name, loadflags, cb, arg);
> -
>
> TRACE("(%p)->(%p,%p,%x,%p,%p): forwarding to IDirect3DRMMeshBuilder3\n", This, filename, name, loadflags, cb, arg);
>
> return IDirect3DRMMeshBuilder3_Load(&This->IDirect3DRMMeshBuilder3_iface, filename, name,
>                                     loadflags, (D3DRMLOADTEXTURE3CALLBACK)cb, arg);

I don't think that's an improvement. "Partial stub" makes a lot more
sense for someone not deeply familiar with the code.

[PATCH 2/8] d3drm: Specify when a method is forwarded to another interface. (resend)

[Christian Costa]

2012/3/14 Alexandre Julliard
> Christian Costa  writes:
> > @@ -438,7 +438,7 @@ static HRESULT WINAPI
>
> IDirect3DRMMeshBuilder2Impl_Load(IDirect3DRMMeshBuilder2*
> > {
> >      IDirect3DRMMeshBuilderImpl *This =
>
> impl_from_IDirect3DRMMeshBuilder2(iface);
> > - TRACE("(%p)->(%p,%p,%x,%p,%p): partial stub\n", This, filename,
>
> name, loadflags, cb, arg);
> > - TRACE("(%p)->(%p,%p,%x,%p,%p): forwarding to
>
> IDirect3DRMMeshBuilder3\n", This, filename, name, loadflags, cb, arg);
> >  return
>
> IDirect3DRMMeshBuilder3_Load(&This->IDirect3DRMMeshBuilder3_iface,
> filename, name,
> >                                      loadflags,
>
> (D3DRMLOADTEXTURE3CALLBACK)cb, arg);
>
> I don't think that's an improvement. "Partial stub" makes a lot more
> sense for someone not deeply familiar with the code.

We forward the method to it's implementation in another interface which is
a "partial stub" (in reality it is more a "partial implementation" as you
will see in next patches).
> From the log we can see a method redirected from interface 2 to interface 3

and then that the method is implemented as "partial stub".
Without the patches I see in the log a partial stub from a trace followed
by a partial stub from a fixme. That's confusing.
I don't see your point with someone novice in this area. Could you
elaborate?

[PATCH 2/8] d3drm: Specify when a method is forwarded to another interface. (resend)

[Alexandre Julliard]

Christian Costa  writes:
> We forward the method to it's implementation in another interface which is
> a "partial stub" (in reality it is more a "partial implementation" as you
> will see in next patches).
> From the log we can see a method redirected from interface 2 to interface 3
> and then that the method is implemented as "partial stub".
> Without the patches I see in the log a partial stub from a trace followed
> by a partial stub from a fixme. That's confusing.
> I don't see your point with someone novice in this area. Could you
> elaborate?

If there's already a stub message in the forwarded function and there's
really nothing else to do in this function then you can get rid of the
partial stub, but adding a trace that essentially describes the
implementation is not useful. It's not like the function is suddenly
going to do something else.

[PATCH 2/8] d3drm: Specify when a method is forwarded to another interface. (resend)

[Christian Costa]

Le 14/03/2012 19:23, Alexandre Julliard a écrit :
> Christian Costa  writes:
> > We forward the method to it's implementation in another interface which is
> > a "partial stub" (in reality it is more a "partial implementation" as you
> > will see in next patches).
> >  From the log we can see a method redirected from interface 2 to interface 3
> > and then that the method is implemented as "partial stub".
> > Without the patches I see in the log a partial stub from a trace followed
> > by a partial stub from a fixme. That's confusing.
> > I don't see your point with someone novice in this area. Could you
> > elaborate?
>
> If there's already a stub message in the forwarded function and there's
> really nothing else to do in this function then you can get rid of the
> partial stub, but adding a trace that essentially describes the
> implementation is not useful. It's not like the function is suddenly
> going to do something else.

So I keep the TRACE but remove the "partial stub" part, right ?
The forwarding information is just a way to make things more explicit 
but I can deduce it from a log so I can live without it.

Christian