#2475 closed patch (Invalid)
pylint daemon.py plus updates
Reported by: | Doadin | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | 2.0.0 |
Component: | Core | Version: | develop |
Keywords: | Cc: |
Description
Takes the pylint rating from 5.53/10 to 9.57.
Attachments (1)
Change History (11)
Changed 10 years ago by Doadin
comment:1 Changed 10 years ago by Doadin
comment:2 Changed 10 years ago by Cas
Please see http://dev.deluge-torrent.org/wiki/Contributing/CodingStyle
Twisted 14 should be backward compatible and if it does break something we will sort it in code at that point, also we would not target newest version of Twisted as it would prevent running on older OSs.
comment:3 Changed 10 years ago by Cas
- Resolution set to Invalid
- Status changed from new to closed
comment:4 Changed 10 years ago by Doadin
Im not sure it breaks anything but running through pylint it states "Module 'twisted.internet.reactor' has no 'addSystemEventTrigger' member" "Module 'twisted.internet.reactor' has no 'run' member" "Module 'twisted.internet.reactor' has no 'callLater' member" "Module 'twisted.internet.reactor' has no 'stop' member"
This is with python 2.7.8 + Twisted 14. However the above has been changed in as far back as 8.2. So i can understand not supporting 14 but how about 8.2 through 13? Maybe it should be changed to just 8.1 only on "Installing Deluge From Source" instead of "twisted >= 8.1".
comment:5 Changed 10 years ago by Cas
Just because pylint complains is not a reason to change the code and afaik from twisted.internet import reactor is the correct way to import the reactor module.
comment:6 Changed 10 years ago by Doadin
Looks like acording to twistedmatrix even for 8.1 it says for twisted.internet.reactor "See twisted.internet.interfaces.IReactor*." I mean apparently it works but my guess is there might be compatibility put in to make it work because its for even 8.1,or importing as just imports everything instead of just the specific part which would work but seems a little unecessary i could go back and look at older versions but i suppose it doesnt really matter if your not gonna change it anyways since it works.
I was just trying to see if maybe some of these changes would have affect on stabability maybe. I thought maybe use of old code from old build tools maybe updating might help or something. Trying to help where i can but i guess you guys have a different idea for the project, deluge is fairly nice as is maybe ill just leave it to the devs.
Edit: Second source of information used http://twistedmatrix.com/trac/browser/tags/releases/twisted-8.1.0/twisted/internet/interfaces.py#L574
comment:7 Changed 10 years ago by Doadin
Honestly at first initial testing the patch has a error anyways so i guess ill just move on.
comment:8 Changed 10 years ago by Cas
All patches that you submit to any project should always be complete and tested: Development/Testing
It is a problem with the way pylint parses Twisted code, not the code itself. The API simply mentions the IReactor link as a reference point, there is nothing there to say that twisted.internet.reactor is deprecated and the Twisted docs show how to setup the reactor in code: https://twistedmatrix.com/documents/14.0.0/core/howto/choosing-reactor.html
comment:9 Changed 8 years ago by Cas
- Milestone changed from 2.0.x to 2.0
Should i just github pull request thing like this i feel like pylint changes dont necessarily have any noticeable affect more so just makes it easier to read. And the updates part is kinda small. Also do you even want thing like this merged? The update part is partly just for the latest version of twisted which on the build page just says >=8 for twisted version but apparently in twisted 14 which is the current version a few things changed. Im guessing the newest version of all the build tools would/should be used in the next build if possible yes?