On Fri, May 8, 2009 at 4:49 AM, ChrisW <email address hidden> wrote:
> Okay, just for clarification, Sidnei's fixes were:
>
> http://zope3.pov.lt/trac/changeset/99805
> http://zope3.pov.lt/trac/changeset/99806
>
> A few comments:
>
> - The original commits in r92767 shows no signs of unit tests being
> written. Why was this?
I believe if you revert r92767 there are tests in Five that break, so
it was a fix for a newer version of Five that got integrated later.
> - r99806 also includes no tests. If this commit was needed, where's the
> test to prove it?
It's a fix for Python 2.4, the test added in 99805 passed on 2.6 but
failed on 2.4
> - Some light testing suggests the patches do fix the issue
>
> - Zope 2 no longer support Python 2.4, so I'm not sure we need to
> special cases for Python 2.4
We don't need to intentionally break it either *wink*.
> I'm setting the issue back to "In Progress" until we get more tests for
> the original changes and r99806.
I will leave it up to you to verify that you're happy with the
response and close the issue then.
On Fri, May 8, 2009 at 4:49 AM, ChrisW <email address hidden> wrote: zope3.pov. lt/trac/ changeset/ 99805 zope3.pov. lt/trac/ changeset/ 99806
> Okay, just for clarification, Sidnei's fixes were:
>
> http://
> http://
>
> A few comments:
>
> - The original commits in r92767 shows no signs of unit tests being
> written. Why was this?
I believe if you revert r92767 there are tests in Five that break, so
it was a fix for a newer version of Five that got integrated later.
> - r99806 also includes no tests. If this commit was needed, where's the
> test to prove it?
It's a fix for Python 2.4, the test added in 99805 passed on 2.6 but
failed on 2.4
> - Some light testing suggests the patches do fix the issue
>
> - Zope 2 no longer support Python 2.4, so I'm not sure we need to
> special cases for Python 2.4
We don't need to intentionally break it either *wink*.
> I'm setting the issue back to "In Progress" until we get more tests for
> the original changes and r99806.
I will leave it up to you to verify that you're happy with the
response and close the issue then.
-- landscape. canonical. com
Sidnei da Silva
Canonical Ltd.
Landscape · Changing the way you manage your systems
http://