[Framework-Team] PLIP #234 Review Revisions

Tom Lazar lists at tomster.org
Thu Feb 12 22:24:19 UTC 2009

On 12.02.2009, at 13:19, Andreas Zeidler wrote:

> On Feb 12, 2009, at 1:05 PM, Andreas Zeidler wrote:
>> [...] i did notice that test (which is why i added "almost" in  
>> "almost none of the changes are actually tested" ;)), but found  
>> that one was far from enough.  anyway, tom will make sure there are  
>> enough now... ;)
> that said i couldn't resist to quickly check the branches for newly  
> added commits...  afaics the only tests you actually added are the  
> ones for the calendar portlet[*], though.  imho, this is not  
> enough.  i think _all_ places affected by the changes in the PLIP  
> need to be tested for correct behaviour for the case when the site  
> root is _not_ the navigation root.
> of course the other case, i.e. having the nav-root _at_ the site  
> root like it is the default in plone, is already well-tested and not  
> the scope of this PLIP anyway.  but i don't think many people have  
> actually used this feature before (as it was partly broken), so we  
> really need more thorough tests for this variant (separate nav-root)  
> now that it has become feasible...

I have reviewed the changes in the documentation and in the three  
affected packages
since december and have conducted another local TTW test.

The test showed that the application of INavigationRoot worked as  
advertised. While I
personally still feel that the switch of the root in the navigation  
tree once you reach
a new root is kind of 'creepy' I realize that a) this is intended and  
b) that in actual
deployment the sub-roots are usually served under different domains  
from the front end
webserver, so the effect won't be observable to users.

While I agree (with andi), that in a perfect world, this PLIP would  
come with more tests, I
also agree (with calvin, optilude etc.) that this PLIP is by nature  
more a bug fix than a
new feature. I don't think we can reasonably expect that all fixes to  
Plone must come with
100% test coverage, or else they will not be accepted.

Seeing that most changes are actually in template markup and not so  
much in code and given
the test of the base portlet and that no existing tests break I vote  
+1 on this.

Can I guarantee that it absolutely won't break anything? No. Am I  
convinced that Plone is better
with this PLIP than without it? Yes :-)

>> cheers,
>> andi
> [*] http://dev.plone.org/plone/changeset/24893
> --
> zeidler it consulting - http://zitc.de/ - info at 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.2.1 released! -- http://plone.org/products/plone/
> _______________________________________________
> Framework-Team mailing list
> Framework-Team at lists.plone.org
> http://lists.plone.org/mailman/listinfo/framework-team

More information about the Framework-Team mailing list