[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r36822 - in trunk: subversion/bindings/swig/python/svn subversion/tests/cmdline/svntest tools/dev

From: Greg Stein <gstein_at_gmail.com>
Date: Mon, 30 Mar 2009 01:35:07 +0200

2009/3/30 Arfrever Frehtes Taifersar Arahesis <arfrever.fta_at_gmail.com>:
> 2009-03-29 21:07:59 Greg Stein napisał(a):
>> On Sun, Mar 29, 2009 at 20:44, Arfrever Frehtes Taifersar Arahesis
>> <arfrever.fta_at_gmail.com> wrote:
>> > 2009-03-29 20:02:25 Greg Stein napisał(a):
>> >> On Sun, Mar 29, 2009 at 19:23, Stefan Sperling <stsp_at_elego.de> wrote:
>> >> > On Sun, Mar 29, 2009 at 05:49:43PM +0100, Greg Stein wrote:
>> >> >> I say it complicated things. More methods instead of fewer. Lots of
>> >> >> additional comparisons instead of just using cmp(). Additional logic.
>> >> >>
>> >> >> And "Python 3.0 compatibility" for scripts intended for 2.4 doesn't
>> >> >> add any benefit.
>> >> >
>> >> > Won't we eventually have to depend on Python 3.x, when Python 2.x is
>> >> > being phased out in a few years or whenever they're planning to do it?
>> >> >
>> >> > At some point, we'll have to do what Arfrever is doing anyway.
>> >> > If we can do some of the necessary conversion work now, and have
>> >> > someone how is eager to do it, even in a way that preserves compatibility
>> >> > with Python 2.x, I don't think it's bad. Especially because converting
>> >> > to Python 3.x does not seem to be trivial.
>> >> >
>> >> > I'm not talking about particular scripts that might well not be useful
>> >> > anymore in a few years (think change-wc-format.py). We don't need to
>> >> > update every line of our Python code. But I don't think there is anything
>> >> > wrong with the general idea of trying to be Python 3.x-compatible as much
>> >> > as possible, even at the cost of temporary extra complexity in the
>> >> > scripts. Eventually, we can drop the Python 2.x code and things will
>> >> > become simpler again.
>> >>
>> >> I agree in principle, and Arfrever's work has been a great help for
>> >> our code. He's been making it more consistent, throwing out the
>> >> *really* old crap, and cleaning up some weirdnesses.
>> >>
>> >> But.
>> >>
>> >> There are some changes (like these past two revs) which just make it
>> >> more complex than a plain 2.x needs to be.
>> >
>> > __eq__() and __ne__() were already used in subversion/tests/cmdline/svntest/wc.py
>> > before these changes, so for consistency they can also be used in
>> > subversion/tests/cmdline/svntest/tree.py and subversion/tests/cmdline/svntest/verify.py.
>>
>> Precedence doesn't mean that the code is simpler. You turned one
>> method into four.
>
> In subversion/tests/cmdline/svntest/tree.py and subversion/tests/cmdline/svntest/verify.py
> __cmp__() was replaced by 2 methods, not 4. I removed UnorderedOutput.__cmp__() which
> was never called.

I'm tiring of this back and forth. You are not persuading me that your
code is any simpler. Backing out the change to tree.py will also fix
the bug you introduced (nodes are no longer ordered). The __cmp__ took
care of that nicely.

> In tools/dev/contribulyze.py __cmp__() was replaced by 4 functions with simple,
> one-line definitions.

And 4 is more than one.

> Anyway, all definitions of __ne__() are very simple.
>
>> Calls to cmp(a, b) turned into something like (a < b) - (b > a). I
>> don't know if that is right.
>
> http://docs.python.org/3.0/whatsnew/3.0.html#ordering-comparisons
>
> A half of '(a > b) - (a < b)' expressions have been already removed.

I disagree with the complexity that you're introducing. I have vetoed
your two changes. You have failed to persuade me that your changes are
an improvement, nor that you've alleviated the complexity concerns
that I have.

I do not see how you're ever going to persuade me that increasing the
method count brings benefit.

Maybe a future patch to fix the hashable thing would be workable. But
these two revisions are not, and I stand by my reasonings and votes.

-g

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1475292
Received on 2009-03-30 01:35:27 CEST

This is an archived mail posted to the Subversion Dev mailing list.