[Zope-CMF] IndexableObjectWrapper

[yuppie]

Hi Miles!


Miles wrote:
> This change is now done, and checked in on a branch.

Great!

> As there are no adapters in CMFCore at the moment, there's no precedent 
> to follow.  So:
> 
>   - The multi-adapter is registered in a new file, implements.zcml

Why did you choose the name implements.zcml? I can't see how 
'implements' describes the contained registration. I would have used the 
content.zcml file or added a catalog.zcml file.

And I would prefer a new marker interface 'IIndexableObject'. I guess 
there are use cases that don't require restricted searchResults based on 
allowedRolesAndUsers and usually we want to catalog more attributes than 
just this one. So we need an interface that marks objects that provide 
all the attributes we want to catalog, not an interface that specifies 
the available attributes.

>   - The TraversingEventZCMLLayer now also loads implements.zcml, as the 
> correct behaviour of both the catalog and the adapter class is required 
> for the folder tests to pass.
> 
>   - 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?

> Otherwise, the branch is here and all the tests pass:
> /Products.CMFCore/branches/miwa-catalog-adapter/Products/CMFCore

Please try to follow PEP 8 and wrap long lines.


Cheers,

	Yuppie