Re: [Framework-Team] Re: PLIP #234 Review Revisions
Andreas Zeidler wrote: [..] i didn't mean to imply that not having more tests is a strict showstopper. please don't get me wrong here. i do appreciate calvin's work and all the fixes he put in to finally make this feature work. hunting the bug must have been cumbersome enough... however, usually we do provide tests to go along with bug fixes as well, to sort of prove things are working correctly now. clearly the case of nav-root != site-root wasn't covered with many tests so far, otherwise we would have seen failures all along. so why not take the opportunity and add a few tests that would have failed without the fixes calvin put in? unfortunately i simply don't have any time left to go through the changesets again and point out tests i'd like to see, but i thought i had already given a generic guideline on what i think is missing. anyway, here's another look at it: if you (temporarily) revert your patches (at least the ones in code), and none of the currently existing tests fail, that's when you should add one that is (and then passes again with your fixes). i don't think that's too much to ask, especially since there are not that many changes (iirc) anyway. but, like i said, i'm running out of time, so i'll leave things to tom and raphael... I hear you ;-) Don't worry; I'm confident Tom and I will handle. Enjoy your vacation, Raphael 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.2.1 released! -- http://plone.org/products/plone/ ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
Re: [Framework-Team] Re: PLIP #234 Review Revisions
On Feb 12, 2009, at 3:10 PM, Raphael Ritz wrote: Martin Aspeli wrote: [..] 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. Personally, I agree with the sentiment that this is mostly a bug fix in nature. +1 While I still plan to take a look on Saturday I won't vote it down for what's being discussed most recently here. i didn't mean to imply that not having more tests is a strict showstopper. please don't get me wrong here. i do appreciate calvin's work and all the fixes he put in to finally make this feature work. hunting the bug must have been cumbersome enough... however, usually we do provide tests to go along with bug fixes as well, to sort of prove things are working correctly now. clearly the case of nav-root != site-root wasn't covered with many tests so far, otherwise we would have seen failures all along. so why not take the opportunity and add a few tests that would have failed without the fixes calvin put in? unfortunately i simply don't have any time left to go through the changesets again and point out tests i'd like to see, but i thought i had already given a generic guideline on what i think is missing. anyway, here's another look at it: if you (temporarily) revert your patches (at least the ones in code), and none of the currently existing tests fail, that's when you should add one that is (and then passes again with your fixes). i don't think that's too much to ask, especially since there are not that many changes (iirc) anyway. but, like i said, i'm running out of time, so i'll leave things to tom and raphael... 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.2.1 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
Re: [Framework-Team] Re: PLIP #234 Review Revisions
Martin Aspeli wrote: [..] 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. Personally, I agree with the sentiment that this is mostly a bug fix in nature. While I still plan to take a look on Saturday I won't vote it down for what's being discussed most recently here. Raphael 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. Martin ___ Framework-Team mailing list Framework-Team@lists.plone.org http://lists.plone.org/mailman/listinfo/framework-team
