[Twisted-Python] jelly with new-style classes: method references are not resolved
Hi, while trying to port jelly to Python 3 (banana tests already pass with Python 3), I found a bug in jelly. It has already been reported 3 years ago with ticket 4935. And there is a fix attached to that ticket. This fix resolves the problem for me. Why has this fix never been included? It is necessary for porting to Python 3. -- Wolfgang
On 11:39 am, wolfgang.kde@rohdewald.de wrote:
Perhaps because the ticket was never given the "review" keyword - so no one ever noticed that there was a potential fix attached to it. See https://twistedmatrix.com/trac/wiki/ReviewProcess Jean-Paul
Am Montag, 1. September 2014, 11:57:22 schrieb exarkun@twistedmatrix.com:
So I should formally be able to review this since I am not the author. What I can say for certain is that the fix looks OK, and porting to Python3 depends on it. Could somebody else please review this ticket 4935? -- Wolfgang
On 12:31 pm, wolfgang.kde@rohdewald.de wrote:
Hi Wolfgang, Your input on the issue would be much appreciated, yes. However, since neither you nor the contributor has commit access, you can't actually grant the ticket a passing review.
I took a look at the patch. There are a couple issues: 1) The tests are not written as unit tests which integrate into the existing test suite. They need to be rewritten so that `trial twisted` will run them. 2) It wasn't immediately obvious to me how the issue is fixed by the patch. It would be nice to either comment on the ticket explaining how the change fixes the problem or add such information in a comment near the code that is changing. Thanks, Jean-Paul
On 11:39 am, wolfgang.kde@rohdewald.de wrote:
Perhaps because the ticket was never given the "review" keyword - so no one ever noticed that there was a potential fix attached to it. See https://twistedmatrix.com/trac/wiki/ReviewProcess Jean-Paul
Am Montag, 1. September 2014, 11:57:22 schrieb exarkun@twistedmatrix.com:
So I should formally be able to review this since I am not the author. What I can say for certain is that the fix looks OK, and porting to Python3 depends on it. Could somebody else please review this ticket 4935? -- Wolfgang
On 12:31 pm, wolfgang.kde@rohdewald.de wrote:
Hi Wolfgang, Your input on the issue would be much appreciated, yes. However, since neither you nor the contributor has commit access, you can't actually grant the ticket a passing review.
I took a look at the patch. There are a couple issues: 1) The tests are not written as unit tests which integrate into the existing test suite. They need to be rewritten so that `trial twisted` will run them. 2) It wasn't immediately obvious to me how the issue is fixed by the patch. It would be nice to either comment on the ticket explaining how the change fixes the problem or add such information in a comment near the code that is changing. Thanks, Jean-Paul
participants (2)
-
exarkun＠twistedmatrix.com -
Wolfgang Rohdewald