Pep8 and more clean-up #980
Conversation
|
Ah and for sure, this should be rebased and merged last before next release ;-) |
|
|
|
|
|
Related to #897. |
|
👍 great work and much needed. Generally I try to be as pragmatic as possible and value readability over strict conformance. When it comes to line length wise I'd vote for the longer length - we've come a long way from teletypes and early video terminals! |
|
Thanks for your input Ross! I am exactly on the same line than you. I submitted a slightly updated version in the latest commit, I propose to everybody to review and comment it |
|
|
1 similar comment
|
|
|
I am with @rozza |
|
As I agree with readability > strict adherence - however I personally prefer the pep8 style of <= 79 characters. Simply because I work from the road so much that when I only have my laptop screen to work with, I will open multiple files side by side (and I shudder when I see horizontal scrolling :P). But at the end of the day if it comes down to it, I can turn on line wrapping. |
|
|
|
@MRigal Can you please rebase so we can merge this and we'll continue this work later on. |
|
For sure I can rebase, but as I said on top, I thought this would be the "last merged PR" before we make the next release |
MRigal
force-pushed
the
fix/various-fixes
branch
from
c90f70d
to
0aeb1ca
Compare
|
|
|
|
Pep8, code clean-up and 0.10.0 changelog finalisation
|
Well done. Thank you! :) |
|
You're welcome, I wasn't the only one to work on it! I have made this release: https://github.com/MongoEngine/mongoengine/releases/tag/v0.10.0 |
|
If you tagged it, it's already on PyPi. |
|
It's not on PyPi because you haven't bumped the version in setup.py. |
|
This should be bumped into 0.10.0 https://github.com/MongoEngine/mongoengine/blob/master/mongoengine/__init__.py#L17 |
|
ufff... release amateur... sorry. I'm not very experienced in it :-) |
|
It's released. |
|
Yes, I saw! Cool :-) |
Hi guys,
Several times I ran into some cases where I came across these init() method returning a value, or some unused statements or variables, as well as many pep8 issues. It is never great to make a big change, but since we are now cleaning so many PRs and there will be anyway a big PR with #946 for next version, I thought it could be a good time to clean generally some code.
I took some time to really read once (quickly) all the code line by line. It allowed me also to find some issues or strange behaviour and to add some TODOs.
I would also like to take the opportunity to tackle one other point. We want to be PEP8 conform and this is very good. However one point is at least vague: line length. We have in our code some pieces which are strictly limited to 79 characters and some others to 119.
PEP8 recommends 79 characters, however Django makes some exceptions there: https://docs.djangoproject.com/en/1.8/internals/contributing/writing-code/coding-style/
Should we include the same point in our contribution guidelines? Some PRs have a lot of code doing only line breaks clean-up (which I tried to avoid in this cleaning effort to avoid unnecessary changes). And at the same time, I see a lot of places in the code where we have line breaks that do make the code significantly harder to read. @rozza @DavidBord @thedrow @seglberg @yograterol can I have your opinion on that point?