[Grok-dev] Re: Proposal to merge ksmith_mcweekly-layers branch
kevin at mcweekly.com
Tue Sep 18 17:03:32 EDT 2007
Thanks for the great feed back. Hopefully I can get to it within the
next few days.
Martijn Faassen wrote:
> 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.
I believe IDefaultBrowserLayer is used as a base interface, but since
it's tied into Rotterdam I'd rather not, with the goal to remove
Rotterdam and possibly replace it with IMinimalLayer or something more
explicitly built-into and available to grok.
> 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?
Good catch, it's can be used in context of both a class and module, I'll
> In meta.py, you have a rather complicated expression:
> # grab layer, if there is one
> view_layer = util.class_annotation(factory, 'grok.layer',
> None) or
> None) or
> 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 martian.util.
Sounds good. In general, the ViewGrokker seems a bit complex whereas the
other grokkers are quite elegant.
> Anyway, only minor comments. With those changes, please, please merge!
> You appear to be missing in the CREDITS.txt file! Add yourself,
> please. :)
> Grok-dev mailing list
> Grok-dev at zope.org
More information about the Grok-dev