On dv, 2008-07-18 at 10:29 -0300, Francisco Tufró wrote:
> So, please, check the last patch and comment about it.
Som-hi! :-)
1. For next patches, remember patching always from the main dir
(not CLAM/). No need to separate patches for different
applications.
2. Since this is a transversal patch with big potential of
conflicts with trunk changes, in next patches, please include
the base revision on which the diff is done.
3. The current patch fails to compile here (using a clean
checkout). Seems a small problem: == Compiling shared
scons/libs/core/src/LadspaWrapper.cxx
scons/libs/core/src/LadspaWrapper.cxx: In member function
‘virtual bool CLAM::LadspaWrapper::Do()’:
scons/libs/core/src/LadspaWrapper.cxx:69: error: ‘class
CLAM::TypedOutControlRegistry’ has no member named
‘GetByNumber_Old’
4. Now let's go for small "commitable" patches in order to reduce
the delta with the big (breaking) patch.
5. A tip about dynamic cast: prefer Type* obj=dynamic_cast<Type*>
over dynamic_cast<type&> because try-catch blocks are very
verbose. As a first version you you could add an assert after
each cast: CLAM_ASSERT(obj, "Expected FloatControl");
6. When changing classes, for example, InControlRegistry ->
TypedInControlRegistry, do not erase the original one. After
doing the switch we'll delete it. But erasing it make the delta
bigger. Files to delete after the switch should be in some TODO
list so we don't forget. So revert those changes in the patch to
reduce the delta.
7. In|OutControl is heavily modified in the patch. Could you
explain the strategy on this?
To conclude: let's now concentrate on produce (and commit) series of
patches to make the trunk closer to the patch. (and also make the
big-patch closer to trunk in cases like erased files)
P