[Grok-dev] Re: Proposal to merge ksmith_mcweekly-layers branch
faassen at startifact.com
Tue Sep 18 15:54:46 EDT 2007
Kevin Smith wrote:
> Wow, alot *has* changed. Yes, please review, a new branch has been
> created based on grok 0.11 trunk, at ksmith_mcweekly-layers-011. Ftests
> are in view/layers.py
Thanks. I just took a look at the code. My feedback:
IGrokLayer is what we inherit from and import from grok. The interfaces
documentation however doesn't describe IGrokLayer but instead talks
about "Layer" as the base interface. I think we should be using IGrokLayer.
Does Zope 3 provide a ILayer base interface? If so, it might be
worthwhile subclassing from it instead of just from interface.Interface,
unless there are problems with this.
The 'grok.layer' directive has a ClassOrModuleDirectiveContext(). The
interfaces.py documentation however claims that grok.layer can only be
used in the context of a class. So which is it?
In meta.py, you have a rather complicated expression:
# grab layer, if there is one
view_layer = util.class_annotation(factory, 'grok.layer',
I'm the type of caveman who will have to scratch his head a few times to
understand what's going down. I suggest rewriting this to a bunch of
'if' statements instead. The same applies to the SkinGrokker. In both
places you're looking for the class annotation and then if that fails
fall back on the module annotation. Factor that into a single utility
function, I'd suggest. The pattern may be common enough to place in
Anyway, only minor comments. With those changes, please, please merge! +100!
You appear to be missing in the CREDITS.txt file! Add yourself, please. :)
More information about the Grok-dev