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

[Alexandre Julliard]

Christian Costa <titan.costa(a)gmail.com> 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.

-- 
Alexandre Julliard
julliard(a)winehq.org

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

[Christian Costa]

2012/3/14 Alexandre Julliard <julliard(a)winehq.org>
> Christian Costa <titan.costa(a)gmail.com> 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 <titan.costa(a)gmail.com> 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.

-- 
Alexandre Julliard
julliard(a)winehq.org

[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<titan.costa(a)gmail.com>  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