[Zope-CMF] Re: Bippity, Boppity, Boo!

[yuppie]

Hi Tres!


Some feedback regarding your notes and checkins:

Tres Seaver wrote on 
http://palladion.com/home/tseaver/obzervationz/cmf_z3_interfaces_20050715:
> 
> I also cleaned up a lot of docstrings in the original interfaces. In many cases, the "style guide" for docstrings has changed over time. My preference has always been for docstrings which specify the contract in the "imperative mode": e.g, rather than:
> 
>     def foo(bar, baz):
>         """ This method frobs 'baz' into the 'qux' method of 'bar'.  'baz'
>         will implement IBaz, unless it is None.  The method raises SpamError
>         if 'qux' returns an error string.
>         """
> 
> I prefer:
> 
>     def foo(bar, baz):
>         """ Frob 'baz' into 'bar.qux'.
> 
>         o 'baz' implements IBaz, or is None.
> 
>         o Raise SpamError if 'qux' returns non-None (an error string).
>         """
> 
> as less chatty, and more clearly "contract".

What about using reStructuredText with Epydoc fields?
See http://epydoc.sourceforge.net/fields.html

The docstring for 'foo' would look like this:

     def foo(bar, baz):
         """ Frob 'baz' into 'bar.qux'.

         :param bar: The bar that will be modified.
         :type bar: an IQuxBar type
         :param baz: The baz that should be frobbed.
         :type baz: an IBaz type or NoneType

         :return: Success indicator.
         :rtype: bool

         :raise SpamError: If 'qux' returns non-None (an error string).
         """

> Remaining Cleanup
> 
>     * Address the XXX comments I scattered through the docstrings, in particular.
>     * Figure out whether we need to leave webdav.WriteLockInterface as a Zope2 interface BBB wart.

There are at least two other dependencies on Zope 2 interfaces:

1.) Code using OFS.ObjectManager.IFAwareObjectManager:

ActionsTool, ActionCategory and TypesTool use IFAwareObjectManager. I 
can have a look at making IFAwareObjectManager aware of z3 interfaces.

2.) Help system:

'interfaces' directories are registered as help directories. If we 
bridge the z3 interfaces back, this should still work.

>     * Decide what to do with the z2interfaces modules / packages to which I moved the discards:
>           o "Bring outcher dead!"? (Just scrag them, that is)?
>           o "None shall pass!" (Reverse bridge them using the new Five directive in Zope 2.8.1)?

I would vote for this option, but the reverse bridge is no Five 
directive (it's just a python function) and so far there are no plans to 
add the reverse bridge to Zope 2.8.1.

Current plans are that the reverse bridge is shipped with Five 1.1 and 
Zope 2.9. There might be other reasons to require Five 1.1 for CMF 1.6: 
I'm currently looking at using its i18n features for the CMF.

On the other hand it would be easy to backport the reverse bridge to 
Zope 2.8.1 if we need it there.

>           o "Bring us ... a shrubbery!" (never mind, I forgot what that was going to be)?

Other issues:

1.) I don't understand why you renamed IActionInfo to 
IActionInformation: ActionInformation doesn't implement this interface, 
ActionInfo does.

Sorry for using similar class names that might have confused you. I try 
to use 'Info' for dicts containing information about an object and I 
hope ActionInformation can be deprecated soon.

2.) Looks like something went wrong with CMFCore.(Old)Discussable: It 
has the wrong Discussable interface and verifyClass got lost.

3.) Why did you add implements() statements like this one:

> -    __implements__ = ( PortalContent.__implements__
> -                     , DefaultDublinCoreImpl.__implements__
> -                     )
> +    implements(implementedBy(PortalContent),
> +               implementedBy(DefaultDublinCoreImpl),
> +              )

AFAICS this is redundant because z3 interfaces are inherited from all 
base classes.

4.) There are still many flags like _isPortalRoot, _isAWorkflow, 
_isPortalContent, _isDiscussable or _isTypeInformation. This could be 
done in an other cleanup, but I think we should use interfaces instead.


Cheers,

	Yuppie