Hash: SHA1

Miles wrote:
> Hi,
>>>>   - The CatalogTool tests set up the adapter at the moment, as a lot of 
>>>> the catalog tests require the adapter to work properly.  This is done in 
>>>> the _makeContent method as it applied to most tests that used the dummy 
>>>> content.  However, I think it belongs somewhere else, but I wasn't sure 
>>>> whether that place was a layer, a setup method or somewhere else.  Any 
>>>> suggestions?
>>> I agree it belongs somewhere else. Maybe a registerWrapper method. But 
>>> can't we make the adapter lookup in catalog_object optional and wouldn't 
>>> that make test setups simpler?
>> Agreed.  I had expected that the catalog would do a queryAdapter, and
>> default to the existing wrapper class if not found.
> Makes sense for BBB - it's possible that someone might be inheriting 
> from the Catalog but not loading the adapter registrations, in which 
> case their code would just break.
> Can I suggest the following logic:
>   1. if the object already implements the IIndexableObject marker 
> interface, no wrapping is required;
>   2. otherwise, adapt to IIndexableObject to do the wrapping;
>   3. if no adapter is registered, fall back to the existing 
> IndexableObjectWrapper class for BBB.

That sounds like what I had in mind, but not for BBB.  I think of the
adapter scheme as a way to choose a non-default wrapper, rather than a
quasi-mandatory replacement for it.

> In the case of 3, I would like to issue some deprecation warning or log 
> message to alert that a registration is required in future.  As 2.2.0 is 
> not yet released, is it possible/desirable to make it a deprecation?

I wouldn't add a warning for it, because I wouldn't rip out the fallback.

- --
Tres Seaver          +1 540-429-0999          tsea...@palladion.com
Palladion Software   "Excellence by Design"    http://palladion.com
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org


Zope-CMF maillist  -  Zope-CMF@lists.zope.org

See https://bugs.launchpad.net/zope-cmf/ for bug reports and feature requests

Reply via email to