[Zope-dev] [Checkins] SVN: zope.publisher/trunk/ Added some BBB code to setDefaultSkin

[Roger Ineichen]

Hi Hanno

> Betreff: Re: [Zope-dev] [Checkins] SVN: zope.publisher/trunk/ 
> Added some BBB code to setDefaultSkin
> 
> Hi.
> 
> Roger Ineichen wrote:
> >> Roger Ineichen wrote:
> >> Then there's some tests and description missing proving 
> that intent. 
> >> All I could read in the changelog and the skinnable tests was 
> >> pointing in the other direction: Making it possible to use the 
> >> Skinnable concept without relying on IBrowserRequest. The whole 
> >> JSONRequest (which is not a
> >> BrowserRequest) tests inside skinnable.txt continue to work.
> > 
> > Yes, I think everything was tested, but probably future 
> ideas are not 
> > documented. There where some discussion about to split each request 
> > into it's own packages.
> 
> Where did that discussion happen? All I have heard of was 
> discussions at PyCon, where nobody quite seemed to see any 
> point in the whole different request types story at all anymore.
> 
> I don't mind if the skinnable story gets less intermingled 
> with the request story in a new zope.skinnable package and 
> breaks some backwards compatibility at that point. Right now 
> that mix of the two concepts is so prevalent in all kinds of 
> places, that I'd rather stay on the backwards compatible side.

Can you tell me what was not backward compatible?

> All ZCML browser directives by default register everything 
> for IDefaultBrowserLayer and expect those resources to be 
> available on "normal" browser requests. The test helpers in 
> zope.app.testing to get browser resources set up rely on the 
> same mix of concerns. This stuff is used all over the place 
> without caring about loading zope.publisher's ZCML right now.

Did my refactoring break anything? If so what?

> >> All I did here was to move two constructs from ZCML into 
> direct code.
> >> The lines I added do exactly the same as the default adapter 
> >> registered as:
> > 
> > Uhh, this is call hard coded and makes it impossible to exclude the 
> > adapter with the <exclude> directive.
> 
> I call that retaining sensible defaults. You can opt-out of 
> the IDefaultBrowserLayer for browser requests by providing 
> your own specialized adapter. I prefer no configuration for 
> the most common case with an opt-out scenario instead of the 
> everything is opt-in behavior.

Do you understand the impact of your changes? I see what you
are trying to do but I don't know which package or application
you are trying to fix with your changes. Can you give me some
hints about what you are trying to fix with your changes?

> > I see two things which are bad. Skinnable depends now on 
> > IBrowserRequest. I moved the skinnable concept out of the browser 
> > request part. This allows us to separate the skinnable and all 
> > different request types into own packages if we do future 
> refactoring.
> 
> zope.publisher is the package that defines the 
> IBrowserRequest interface. It might make sense to split those 
> concerns off into different packages and then straighten out 
> the dependencies. At that point I can see having a 
> setDefaultSkin method inside zope.skinnable with a different 
> behavior. But the one inside zope.publisher ought to play 
> nicely with IBrowserRequest.

Any improvements are very welcome. Do you think we should move
the skinnable concept into zope.skinnable?

Sorry if I'm bother you about this details but I spent
a full day with this refactoring and one of my apps depends
on this deault browser layer less concept. You just reverted
the hole refactoring with 3 lines of code.

Regards
Roger Ineichen

> Hanno
> 
> _______________________________________________
> Zope-Dev maillist  -  Zope-Dev at zope.org
> http://mail.zope.org/mailman/listinfo/zope-dev
> **  No cross posts or HTML encoding!  ** (Related lists -  
> http://mail.zope.org/mailman/listinfo/zope-announce
>  http://mail.zope.org/mailman/listinfo/zope )
>