[Twisted-Python] [twisted.roundup@twistedmatrix.com: [issue291] app.getApplication cleanup]

okay, and now someone explain to me what should motivate me to keep sending in patches? strports patches will be rejected too, moshez told me, so i'll have to fork that too.. and i had fixes for that loseConnection stuff months ago, i don't remember if i submitted them, because even back then i knew from experience that you won apply my patches, but i probably did. you realize that you are unwilling to apply any of my patches, even when minor, and when i'm trying really hard to get them in? it would be really easier to maintain a patch dir, and just fix my own cvs checkout locally. i'll submit my proxy patches, integrating well into cvs-current, this week, as arranged on irc, and you judge if my code is unbearable or if it actually makes sense. i see this as an last attempt to communicate with you guys.. bitter, paul ----- Forwarded message from Moshe Zadka <twisted.roundup@twistedmatrix.com> ----- Subject: [issue291] app.getApplication cleanup To: paul@soniq.net From: Moshe Zadka <twisted.roundup@twistedmatrix.com> Reply-To: Twisted issue tracker <twisted.roundup@twistedmatrix.com> Date: Tue, 30 Sep 2003 05:40:22 +0000 Moshe Zadka <moshez@twistedmatrix.com> added the comment: I'm going to reject it -- this code works, and is clear enough. Do you have any good reason, other than clean-up, for this patch? ---------- status: unread -> chatting __________________________________________________________________ Twisted issue tracker <twisted.roundup@twistedmatrix.com> http://www.twistedmatrix.com/users/roundup.twistd/twisted/issue291 __________________________________________________________________ ----- End forwarded message -----

On Tue, Sep 30, 2003 at 01:28:21PM +0200, Paul Boehm wrote:
okay, and now someone explain to me what should motivate me to keep sending in patches? strports patches will be rejected too, moshez told me, so i'll have to fork that too..
Moshe is the maintainer of that particular section of code; it would take a lot to convince other developers to override his judgement on those issues. Certainly, a cosmetic patch such as this particular one (which merely rearranged code, even if it does seems clearer) isn't something to get particularly bothered about either way. Also, here's what moshez said regarding strports patches (according to my IRC log): <moshez> patches to make "strports more general" will be rejected until I see a use case. we tried to come up with a use case and didn't have success Your paraphrase above ("strports patches will be rejected too, moshez told me") implied something quite different to what he actually said (please correct me if he said otherwise and I didn't see it!). He requires use cases to justify adding more features -- presumably if you want to add a feature, you will have a reason, so I don't see this as being particularly onerous :)
and i had fixes for that loseConnection stuff months ago, i don't remember if i submitted them, because even back then i knew from experience that you won apply my patches, but i probably did.
I've searched the issue tracker and the mailing list, and can't find what you're referring to. Please post it to the issue tracker, so that we can ignore it properly ;) Seriously, it looks like you haven't sent that patch to us, so please do.
you realize that you are unwilling to apply any of my patches, even when minor, and when i'm trying really hard to get them in?
I know some people feel that cosmetic patches that merely change code to look prettier are more risk than they're worth; e.g. when True and False was added to Python, Guido discouraged people from going through and making the obvious cosmetic changes in the standard library because of the risk of introducing bugs. If a maintainer feels that the cost of applying and encouraging cosmetic patches outweighs the small (and ultimately subjective) benefit they give, then they will reject them. I'm not sure of Moshe's specific reasons for this rejection, beyond what he wrote in the issue tracker: "I'm going to reject it -- this code works, and is clear enough." But judging from that, his point of view is similar to what I just described. He didn't see that patch as offering a substantial enough improvement over the existing code to be worthwhile. Extrapolating from this one minor patch to "you are unwilling to apply any of my patches" seems a little premature. Are there others from you that have been rejected that I don't know about?
i'll submit my proxy patches, integrating well into cvs-current, this week, as arranged on irc, and you judge if my code is unbearable or if it actually makes sense. i see this as an last attempt to communicate with you guys..
I'm listening, and I believe the other developers are too. Some of us are unnecessarily abrasive at times (yes Moshe I mean you), and I regret that. I look forward to seeing your proxy patches and discussing them with you. Thanks for your efforts so far (even though they have gone unrewarded)! Regards, -Andrew.

I had the same conversation with moshe about making strports more general, and like he said, we couldn't find any uses cases. To emphasize the point, *I* couldn't find any, and I was asking for these changes. So we decided not to bother changing it until we do. If you do have a use case, then I'm sure extending it won't be a problem, and moshez was quite willing to change the code for that case. But we're all working on this in our spare time, it's not our job, so in order for us to do extra work there has to be a reason. YAGNI is a valid argument. -- Itamar Shtull-Trauring http://itamarst.org/ Available for Python & Twisted consulting

Paul Boehm wrote:
okay, and now someone explain to me what should motivate me to keep sending in patches? strports patches will be rejected too, moshez told me, so i'll have to fork that too..
I don't know. What motivated you to send patches in the first place?
and i had fixes for that loseConnection stuff months ago, i don't remember if i submitted them, because even back then i knew from experience that you won apply my patches, but i probably did.
As Andrew pointed out, there is nothing on the list or tracker. Certainly you'd learn from experience that if you don't submit patches, we won't apply them.
i'll submit my proxy patches, integrating well into cvs-current, this week, as arranged on irc, and you judge if my code is unbearable or if it actually makes sense. i see this as an last attempt to communicate with you guys..
You may want to consider trying a little harder before giving up. You make this sound like a last resort after you've spent a long, hard time explaining yourself and lobbying for an important patch. However, this is a trivial patch, "cleanup" which isn't required unless you happen to find for loops substantially easier to read than list comprehensions (or longer functions easier to read than shorter ones). Looking at issue 291, the conversation goes: Moshe:
I'm going to reject it -- this code works, and is clear enough. Do you have any good reason, other than clean-up, for this patch?
Paul:
no besides i've joined the dark circle now and understand the code anyway, lets keep those pesty newbies away from the secrets of the inner circle!
This doesn't look like a lot of hard arguing to get bitter over. Moshe asked you for reasons and you didn't give him any. There is good reason not to accept patches which simply shuffle things around. Every change has a cost. Every change is a small bit of functionality which will be subtly different between versions. We are striving to create as few of these as possible, and any long-time Twisted user can still tell you that we create a LOT. Any bug report involving the code in question will automatically get longer, because instead of dealing with the only released version, the developer will always have to ask "Is this with 1.1.1 or 1.1.2?" So, if you want "cleanup" changes to be accepted, you are automatically fighting an uphill battle, and for good reason. If you really feel that aesthetic changes like this are important, your best bet is to include some useful modification to the current file - preferably with a strong use-case attached in the tracker or in docstrings, as well as tests - and it is very unlikely that we will reject patches on the grounds that they contain additional cosmetic changes, especially if those changes are related to the fix or enhancement. I'd still prefer you avoid this kind of change, but working, useful code is always a good bargaining chip. Also, just in terms of priorities: if readability is your goal your energies would best be directed elsewhere. May I humbly suggest that you examine other parts of Twisted, such as twisted/spread/jelly.py, twisted/protocols/imap4.py, or this lovely snippet from twisted/protocols/dns.py:
byte3 = (( ( self.answer & 1 ) << 7 ) | ((self.opCode & 0xf ) << 3 ) | ((self.auth & 1 ) << 2 ) | ((self.trunc & 1 ) << 1 ) | ( self.recDes & 1 ) ) byte4 = ( ( (self.recAv & 1 ) << 7 ) | (self.rCode & 0xf ) )
As far as readability goes, you can do a lot worse in Twisted than any of the code in twisted/application.
i see this as an last attempt to communicate with you guys..
bitter, paul
Sit back, take a deep breath, and relax. We have no prejudice against you or your patches. Each one will be evaluated in turn. If the maintainer disagrees with you, or even if not, you may still be expected to give coherent and complete examples of why the patches are important enough to justify their cost as changes, but we turn down relatively few patches overall. If you're having trouble communicating, keep the discussions to public, archived forums like this mailing list or the particular issue in question, so that others might help you argue your case. Unless you know another person's style of communication well, IRC can be an exercise in frustration, and the Twisted developers' style includes many subtle hints about tone of voice which are not immediately obvious to those outside the group.
participants (4)
-
Andrew Bennetts
-
Glyph Lefkowitz
-
Itamar Shtull-Trauring
-
Paul Boehm