Linting and cleaning #1145
Linting and cleaning #1145
Conversation
Linted with flake8, ignoring a handful of items which seemed to be less crucial F403,E266,F401,E402,F405 I was inspired to do this work because of all of the hanging whitespace. When I saw the contributor docs are looking to move to PEP8, I wanted to contribute some effort to clean up the codebase. If this work is appreciated and merged, I'll move into the subdirectories as I have time.
Codecov Report
@@ Coverage Diff @@
## master #1145 +/- ##
==========================================
- Coverage 83.51% 83.47% -0.05%
==========================================
Files 157 157
Lines 37818 37816 -2
==========================================
- Hits 31585 31568 -17
- Misses 6233 6248 +15
|
|
Hi ! |
|
The codecov is a little spurious, the - 0.05 coverage delta is most likely due to a denominator increase where I added the newlines that PEP8 requires between top-level Let me know if I should worry about this. |
|
As explained by @gpotter2 and in CONTRIBUTING, we try to keep the code history as possible. Sorry about that. |
|
@p-l- Sorry, I'm quite confused. The robot says you're trying to keep the codebase clean. I've cleaned this code. It's now more clean. You say "we try to keep the code as history as possible". I'm having a parse failure on that sentence. I had a lot of difficulty reading scapy source code because of all of the trailing whitespace and inconsistencies in style. Your CONTRIBUTING guides says you're pushing for PEP8. What am I missing? Cheers! |
|
@jcrowgey He surely meant « as clean as possible ». We know that there are tons of non Pep8 files in the code. If you read arch/pcapdet.py for instance, the file is absolute madness. But if we pepify all the code, any ˋgit blame` we will do will point to the pepify commit, which would make debugging much harder... |
|
Lol @gpotter2 is not a robot (or are you, @gpotter2? That would explain how hard you can work)! I'm really sorry if you have troubles reading Scapy source code because of trailing whitespaces, but we won't break the code history for that. When working on Scapy I really often use Also, if you read CONTRIBUTING, you'll see that the PEP-8 compliance statement applies to new code. BTW, maybe next time, before writing your pull request, you could open an issue to tell us you're working on something (that's also suggested in CONTRIBUTING). This would allow us to discuss before you do an important work and avoid the frustration (that I totally understand; again, I'm sorry). |
|
Applying PEP08 to the whole Scapy source is on our wish list. However, we don’t know how to do it while preserving our constraints. Among others, these ones are currently important:
- keep the git history clean and useful
- don’t promote new commiters with simple code changes
In a perfect world, we could use a tool to lint the code and preserve our constraints. As of today, it does not exist and might be difficult to develop.
Be sure, that having a not so clean source make sometimes our job more difficult =)
If you wish to be involved in Scapy development, have a look to at #399
|
|
Thanks, everyone for the fuller explanation. Sorry you guys aren't in a state where you can accept this work. Sorry, @gpotter2, for dehumanizing you. Your prompt response and what seemed (from my point of view) to be a somewhat non-sequitur reply made me assume that I was looking at an automated message gone wrong. |
Linted with flake8, ignoring a handful of items which seemed to be less
crucial F403,E266,F401,E402,F405
I was inspired to do this work because of all of the hanging whitespace.
When I saw the contributor docs are looking to move to PEP8, I wanted to
contribute some effort to clean up the codebase.
If this work is appreciated and merged, I'll move into the
subdirectories as I have time.
I created a corresponding issue: #1144