Re: [Framework-Team] PLIP #126 ready for review
David Glick wrote: PLIP 126 (Link type should automatically redirect when accessed directly) has been implemented. You can get the review buildout from: http://svn.plone.org/svn/plone/review/plip126-link-redirects I'm going to paste the contents of http://svn.plone.org/svn/plone/review/plip126-link-redirects/PLIP_126_README.txt here for ease of any discussion... Review bundle for PLIP 126: Link type should automatically redirect when accessed directly == As Danny triggered some discussion already I'll add my comments (which I don't consider finished yet - that's why I didn't post them up to now). It's been a while that I did this and I might not be up-to-date with everything. Raphael's comments == (as of 2009-01-16) * on a new site the link type and the configuration options work as advertised * the migration worked for me on a randomly picked site of mine. * I don't understand why 'link_view' isn't made available as an alternative view on instance base still. That could easily be achieved by leaving it in the Available view methods property of the FTI. That way one could control this behavior on instance base even. At the very least this should be properly documented. * running the tests for CMFPlone I get 3 failures but they are all unrelated to the changes in question here (UnicodeSplitter, Latin1 processing, and migration related stuff) * Running the tests of plone.app.controlpanel on a vanilla checkout of the plip buildout generated one failure for me (Ubuntu/packaged uncustomized Python 2.4): ... Set up Products.PloneTestCase.layer.PloneSite in 18.665 seconds. Running: . Failure in test /home/ritz/buildouts/reviewing/plip126-link-redirects/src/plone.app.controlpanel/plone/app/controlpanel/tests/types.txt Failed doctest test for types.txt File /home/ritz/buildouts/reviewing/plip126-link-redirects/src/plone.app.controlpanel/plone/app/controlpanel/tests/types.txt, line 0 -- File /home/ritz/buildouts/reviewing/plip126-link-redirects/src/plone.app.controlpanel/plone/app/controlpanel/tests/types.txt, line 75, in types.txt Failed example: self.browser.contents Expected: '...Globally addable... ...Allow comments... ...Visible in searches... ...input id=redirect_links... ...type=checkbox... ...name=redirect_links:boolean... ...checked=checked... ...label for=redirect_links... ...Redirect links to remote url...' Got: '\n \n\n\n!DOCTYPE html PUBLIC\n -//W3C//DTD XHTML 1.0 Transitional//EN\n http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd;\n\n\nhtml xmlns=http://www.w3.org/1999/xhtml; xml:lang=en\n lang=en\n\n (truncated by me here) Guess it's because it expects ...Redirect links to remote url...' but it gets ...Redirect immediately to link target... Potentially more to follow Raphael ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] PLIP #240 ready for review
Erik Rose wrote: I actually first implemented it exactly that way (even called it IRefreshableLockable), then wondered if the complexity was worth it. I can go either way, but would like to hear some other opinions of best practice in a case like this. For what a non-3.x-FWT opinion is worth, I'd rather program against a proper IRefreshableLockable interface. Or, if nobody's implementing it in the wild, it'd be even better (less complex) to revise ILockable. I'd recommend to stay away from revising ILockable as this isn't necessarily even Plone specific. IIRC Jeff and I where trying to follow the at-the-time default way in which Zope handled DAV locks when we wrote this. Raphael Cheers, Erik ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] Re: PLIP #239 ready for review
On Jan 13, 2009, at 12:05 AM, Martin Aspeli wrote: Martin Aspeli wrote: I've just finished the base implementation of PLIP #239, adapterise the ExtensibleIndexableObjectWrapper, for your review. A review buildout can now be found here: https://svn.plone.org/svn/plone/review/plip239-indexer i've added my review notes in http://dev.plone.org/plone/changeset/24712 in short: no comments, +1 for merging. and i'd recommend reading the doctest to everyone! it makes a great example of how valuable they can actually be... cheers, andi -- zeidler it consulting - http://zitc.de/ - i...@zitc.de friedelstraße 31 - 12047 berlin - telefon +49 30 25563779 pgp key at http://zitc.de/pgp - http://wwwkeys.de.pgp.net/ plone 3.1.7 released! -- http://plone.org/products/plone/ PGP.sig Description: This is a digitally signed message part ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
