Calvin Hendryx-Parker wrote:

I'd be open for suggestions for other tests. A majority of the fixes were templates and they were to use an already existing bit of code functionality that I didn't add like you said. The viewlets were modified to support this and I have the test to confirm it in the base class for the viewlets.

If there are some specific tests you'd like to see I'd be happy to add them, but in the end the trickiest piece of code I updated was the calendar tool modifications and I added tests there also.

FYI... we have released collective.lineage which depends on many of these fixes and I believe will expand the number of people using this functionality. There are a lot of people excited about this new product and are wanting to deploy it. We actually have 1 site in production with it already. Since this functionality was already in Plone before I got ahold of it I still see this PLIP as a large bug fix that really should be included.

FWIW, I think much of Calvin's work could've gone into a 3.2.x release as bug fixes. If he doesn't break tests, and writes a few tests for truly new code, then I think that's probably sufficient in most places.

Having done something similar in the past (but not read Calvin's diff in detail), I suspect most of his changes were simply to stop people making use of portal_url() when they should've used get_navigation_root(). There may be cases when we could add a "defect" type test to show that the navigation root didn't work before, but now does, but let's not create too much work for what is, in many cases, more about analysing a problem and applying a few surgical fixes, than writing a ton of new code.


Author of `Professional Plone Development`, a book for developers who
want to work with Plone. See

Framework-Team mailing list

Reply via email to