[Python-Dev] Encouraging developers

[Josiah Carlson]

Giovanni Bajo <rasky at develer.com> wrote:
> On 3/6/2007 3:11 AM, Josiah Carlson wrote:
> > Giovanni Bajo <rasky at develer.com> wrote:
> >> I think this should be pushed to its extreme consequences for the standard 
> >> library. Patching the standard library requires *much less* knowledge than 
> >> patching the standard core. Basically, almost any Python developer in the wild 
> >> can quickly learn a module and start patching it in a few days/weeks -- still, 
> >> the stdlib is a total mess of outdated and broken modules.
> > 
> > Sometimes code is correct.  Sometimes code is complete.  Just because a
> > module hasn't been significantly altered or updated recently doesn't
> > mean that it is outdated or broken.  
> 
> You can't decide this unless you are a maintainer of that module. If a 
> programmer felt the urge to pack up a patch and submit it to SF.NET, he 
> probably has an idea. It might be a good idea. It might not be the best, 
> but at least it's an idea. It might even be just a bugfix.
> 
> If you see the patch, know the module well, and can express a judgement, 
> you just need to review it and discuss it. But I really can't see what's 
> *worse* than getting a patch unreviewed and unapplied.

And who is advocating for patches to be unreviewed and unapplied?  I'm
not.  If *anyone* is advocating such a position, then I claim that they
are idiots.

>  > Asyncore, for example, is more or
> > less feature complete for a minimal asynchronous socket framework. Could
> > it gain more features (like good SSL support) or have better error
> > handling?  Of course.  But it passes the test suite (via test_asynchat),
> > so I would consider it *not* broken.
> 
> Because you are the maintainer and you know well. I used asyncore once, 
> and I felt it was incomplete. So I went looking for something else. 
> That's fine. You probably know asyncore very well, so your judgement is 
> important and you'll be reviewing the patches and vetoing them if you 
> don't like.

I'll tell you the biggest problem with asyncore: there are few *good*
samples of asyncore usage in the wild, and there isn't even one really
in the standard library (smtpd does OK, but it could be better).  The
asynchat module is supposed to add *just enough* functionality to get
people started, but its lack of a sample collect_incoming_data() and
found_terminator() that generally do the right thing, are sticking
points for many people.

Among the changes I'm going to be pushing for is the inclusion of sample
implementations of those two methods in asynchat (as well as a fix for
when a string terminator is longer than ac_in_buffer_size).  If asynchat
had them in the summer of 2001, I probably wouldn't have more or less
reimplemented my own asynchronous sockets framework that summer and fall
3 different times.


> But there are modules without maintainers. We agree that every module 
> should have a maintainer, and that's fine. But you seem to ignore the 
> fact that we ought to find a solution for modules and packages without 
> maintainers. What is your proposed solution, once if we agree that the 
> current state of affairs suck? You can't force people to step up for 
> maintainership of course, so you have to find something in the middle. 
> And the best way to encourage someone to maintain a package is... 
> accepting their patches.

And that's what I think is bull.  Whether or not we want or need
maintainers for module or package X is independant of the fact that user
Y has submitted a patch for it.  If they say, "I would like to become
maintainer of package X", ok, if they are better than not having a
maintainer, great.  But to ask them to be a maintainer of an
unmaintained package because they submitted a patch? That's a bit like
inviting everyone who has ever programmed to be an IEEE or ACM fellow.
That's nice and inclusive, but not the way you gather together people
who can and will develop quality code.


> > Just because a patch doesn't break a module, doesn't mean the patch
> > should be applied.  Take, for example, any one of the patches currently
> > offered against asyncore.  One of them more or less changes the internal
> > structure of the module for no other reason than to (in my opinion) be
> > gratuitous.  Certainly it has some good ideas, but it would be better to
> > adapt portions rather than take it completely - which is what you are
> > suggesting, and which is what would have happened due to the absence of
> > an asyncore maintainer (until I took it up).
> 
> Engineering is a matter of tastes and opinions, more often than not. 
> Once you are a maintainer, your taste wins. But if there are no 
> maintainers, I prefer to trust someone who wasted his time to produce a 
> patch, rather than just blatantly ignore his job. At least, he had an 
> urge and produced some code. He did put forward his opinion.

Certainly opinions differ, which is what this is.  However, even
high-quality patches (like the one produced by Larry Hastings for string
concatenation and views) are rejected because the functionality is
deemed undesireable.  If *any* of the recent changes that were rejected
were engineered, it's that one.  But not all patches are engineered, and
certainly not all patches implement functionality that is desireable.


> Moreover, you're debating my arguments speaking of refactoring patches 
> or new features. You're ignoring that bug-fixes go often unreviewed, and 
> sometimes even some basic new feature with very small patch which really 
> can't do *much* harm.

I disagree.  Not every feature that a user thinks should exist, should.
Too many people are often too close to the problem to understand that
their solution simply sucks.  I've written more code than I care to
admit that solved a problem in a particular way, which I built upon over
months, that I realized later was a *horrible* design.

Maybe this is a similar case.  I personally don't find Python's
development process as "broken" as some would claim, primarily because
Python works for me (and I'm using the no longer maintained 2.3.5).  Yes,
I would really like a subclassable mmap.  Yes, I would really like
binascii.long2bin() and bin2long().  


> For instance, I did a small patch to zipfile which adds decryption 
> support (to open zipfile crypted with password, with the standard zip 
> encryption algorithm). It's still unreviewed somewhere in SF. I wonder 
> how much harm could have be done if that patch had been simply applied 
> as-is years ago. A little game: without looking at this patch of mine, 
> how much are you willing to bet that committing that patch is going to 
> cause pain the Python community and other stdlib maintainers, to cause 
> headaches like deprecations because of broken interfaces, and whatnot?

It's a toss-up.  Considering my limited knowledge on zipfile handling, I
would say no.  In the realm of language design, standard library
modification, etc., when it comes to Python, a conservative approach is
the best approach.  Changing things, just to change them, is a horrible
idea.  Changing things, because the change is positive with no overtly
negative effects, is a great idea.  Never mind that classic zipfile
encryption is broken when there are more than 3 files in the archive,
and there are 2 incompatible encryption rutines in modern zip variants,
and one could make the claim that accessing files that claim to be
secure, but really aren't, is a misfeature (which is also why the rotor
module was removed).


> In other words, I maintain that if my patch had been applied 3 years ago 
> without even looking at it, zipfile would be a better module now. It 
> might not be the best possible solution, it might have some little bug, 
> it might expose a new interface which is incomplete and might be 
> deprecated in the future if more work on encryption support will be 
> done. Heck, I don't even remember the patch myself. But *meanwhile*, I 
> counted several requests on comp.lang.python for such a feature in these 
> years, and my patch is simply getting dust.

And you are entitled to your opinion.  I think you are full of it, and
thankfully, in this context, other people tend to agree that applying
user patches without them being reviewed is a foolish idea.


> Remember that also maintainers do mistakes, so it really can't be much 
> worse than just ignoring any patch.

I believe that ignoring patches is better than accepting without review.
Then again, I've received and reviewed some *horrible* patches, and am
pessimistic about the quality of submissions until the submitter has
proven themselves.  Also, I'm a jerk.


 - Josiah