[1/18] d3dx9: Add shader assembler private definitions

Juan Lang juan.lang at gmail.com
Mon Aug 17 17:32:47 CDT 2009


> Well, to be true all the first 12 patches add code which is
> effectively used only from patch 13 onwards. But I couldn't find a
> better way to split the patches in manageable pieces while avoiding
> these kind of issues.

I tend to agree with Vincent here:  adding a bunch of dead code that
all gets enabled at once can make it rather hard to pinpoint the
source of regressions.  Your patch numbering is wrong, as you know, as
your patch 4 belongs between patches 1 and 2, so that alone merits a
resend.  But I think the order is rather backwards anyway.  The
typical sequence is,

1) Change any public interfaces you need to.  I think your patch 14
falls into this category.
2) Add stubs for the public functions or interfaces you want to add.
For instance, you'd add stubs for D3DXAssembleShader,
D3DXAssembleShaderFromFile and D3DXAssembleShaderFromResource either
as one patch--often acceptable when adding stubs--or as separate
patches.
3) Add tests for the newly exposed functions or interfaces,
appropriately marked with todo_wine.
4) Incrementally add code to implement the functions or interfaces
you're implementing, removing todo_wine from any tests that begin to
pass as a result.

Step 4) really comprises many small patches, and will often comprise
the bulk of the patchset.  It's important to make each commit atomic
and useful.  So, for example, adding a bunch of declarations that
aren't used to a private header isn't useful, though it is atomic.
You'd only want to add to a private header declarations which are used
by code in the same patch.  So your patch 1 should be both split and
combined with other patches from your series.

I hope my general directions make some sense.

The difficulty with the patchset as it is is that it requires a great
deal of knowledge and a prodigious memory on the part of the reviewer.
 The reviewer must read and understand the entirety of your
implementation before he can begin to understand how it's used.  For
superhumans like Alexandre perhaps this is reasonable, but for mere
mortals like the rest of us, more easily digestible pieces are far
easier to understand.

Thanks,
--Juan



More information about the wine-devel mailing list