Comments (7)
I agree, less pylint errors/warnings will greatly benefit the code base. Yet, pylint has a fairly strict default configuration, so before we start fixing things that are not broken, we should agree on what we want pylint (not) to check.
Let us take dispersy.py as an example. The following are a few [C]onvention for coding standard violations that occur:
-
C: 2,0: Line too long (99/80)
The default limits lines to 80 characters, I propose we increase the limit either to 100 or 120 characters, see http://richarddingwall.name/2008/05/31/is-the-80-character-line-limit-still-relevant/
-
C: 1,0: Too many lines in module (4631)
The default limits files to 1000 lines, I propose we either increase this or ignore it all together
-
C: 86,0:SignatureRequestCache: Missing docstring
Good docstrings are very important, however, bad docstrings are even more damaging. Forcing programmers to include a docstring will (in some cases) result in pointless docstrings (i.e. set_value(value) with docstring 'sets the value') or placeholder docstrings (i.e. 'foo bar, todo doc'). Until pylint can distinguish between cases where a docstring would be usefull, I believe it is better to disable this warning.
-
C:137,22:MissingSomethingCache.init: More than one statement on a single line
Occurs very frequently in dispersy code when debug output is given in the form
if __debug__: dprint("signature timeout")
. My personal preference is to keep it on a single line to improve code readability.
There are a few [W]arning for stylistic problems or minor programming issues that we should discuss as well:
- W:852,15:Dispersy.reclassify_community: Used * or ** magic
- W:1380,78:Dispersy._...: Used builtin function 'map', 'filter', ...
- W:2903,42:Dispersy._send: Unused argument 'value'
from dispersy.
- C: 2,0: I think we should limit the line size, I agree on the 100~120 limit.
- C: 1,0: I think we should refactor the code a bit, maybe 4434 lines for a single class is a bit excessive. I see a decent part of it are asserts and docstrings, but still... I think having it split would help newcomers wrap their head around the thing.
- C: 86,0: I don't think its that bad to have " foo accessor " docstrings and we should be responsible enough to not put placeholder strings (It's not necessary to add a TODO docstring if pylint will remind it to us anyway)
- C:137,22: Does everyone agree on disabling this one?
from dispersy.
We will add a pylint.rc file to the repository with the settings that we agree on. After a discussion with Elric we came with the following:
[MESSAGES CONTROL]
disable=C0321,W0142
[FORMAT]
max-line-length=120
max-module-lines=3000
[BASIC]
no-docstring-rgx=_.*
from dispersy.
I've added the config file to jenkins, Ill be progressively updating the projects to use it.
Right now only Test_dispersy_devel is using it.
I will also periodically update the number of violations that mark a build as failure and no pull requests that go over the threshold should be merged.
from dispersy.
autopep8 can be run from console to fix many problems in one go. I suggest running this on all code, running it locally before doing a merge will prevent merge problems occurring.
/usr/local/bin/autopep8 --in-place --max-line-length=120 --jobs=0 --recursive --pep8-passes=3 .
from dispersy.
/usr/local/bin/autopep8 --in-place --max-line-length=120 --ignore=E501 --jobs=0 --recursive --pep8-passes=3 .
Ignoring E501 will leave line lengths alone. This results in less invasive changes, while still doing the changes that Eclipse does.
from dispersy.
I guess we can close this one now.
from dispersy.
Related Issues (20)
- Test test_tracker in TestBootstrapServers is unstable HOT 1
- The upgrade procedure is not tested and goes back to version 1
- Generator only used to get one item? HOT 2
- TaskManager tests are hanging. HOT 2
- Refactor _resume_delayed HOT 1
- The libnacl submodule can be removed
- The add and add_keys in the bloomfilter contain identical code
- The test to check whether the bootstrap servers are up should be fixed
- Get rid of all the hard-coded sleeps in the tests
- meta_message table is filled with duplicates
- Linux unit tests pass on PR with error HOT 1
- Error when changing meta messages on community upgrade HOT 1
- The Dispersy tracker stress test could be part of our tests HOT 1
- Make Dispersy available on PyPi HOT 3
- Remove DoubleMemberAuthentication
- Bootstrap server test does not work anymore after n-hop sync HOT 3
- Infinite growing dictionary in Dispersy statistics HOT 2
- Add new release HOT 1
- Missing license HOT 2
- Speeding up torrent downloads in Tribler - Peer Video Club HOT 3
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from dispersy.