Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Format code according to Python recommendations - PEP 8 #37

Closed
goodrone opened this issue Aug 4, 2013 · 14 comments
Closed

Format code according to Python recommendations - PEP 8 #37

goodrone opened this issue Aug 4, 2013 · 14 comments
Assignees
Labels
Milestone

Comments

@goodrone
Copy link

goodrone commented Aug 4, 2013

Since Mailpile claims to be a piece of software related to security, its source code should be widely reviewed. Please adhere to well-established standards of writing Python code.

http://www.python.org/dev/peps/pep-0008/
http://www.pylint.org/

@pmav99
Copy link

pmav99 commented Aug 4, 2013

+1. PEP8 is practically universally accepted.

Even if you don't adopt PEP8, you should define/document your coding standard conventions in order to allow contributions to be consistent with the existing code base.

@anthonyfinch
Copy link

+1 I would be strongly in favour of PEP8 also.

@markberger
Copy link

+1

@kmike
Copy link

kmike commented Sep 5, 2013

+1

Could please someone from core Mailpile developers take a look at this? External contribution is not easy because the diff would be huge, and there is even a security risk because of that.

@BjarniRunar
Copy link
Member

We are going to slowly move in this direction. New source files will be PEP 8 compliant and we'll be cleaning up the old ones a little bit at a time.

@ghost ghost assigned BjarniRunar Sep 19, 2013
@vibragiel
Copy link

Since @markberger has said he doesn't have enough time to keep working on this issue, I'd be glad to take over and help making old files compliant with PEP8 (there's also some pyflakes cleanup to be done). I agree with @kmike that this is going to produce huge diffs, so maybe we could do it little by little, on file at a time. What do you think?

@markberger
Copy link

After formatting the one file, I would recommend committing indentations
and code changes seperately. That way it is clear what lines of code have
been changed.

On Thursday, September 19, 2013, Gabriel Rodríguez Alberich wrote:

Since @markberger https://github.com/markberger has said he doesn't
have enough time to keep working on this issue, I'd be glad to take over
and help making old files compliant with PEP8 (there's also some pyflakes
cleanup to be done). I agree with @kmike https://github.com/kmike that
this is going to produce huge diffs, so maybe we could do it little by
little, on file at a time. What do you think?


Reply to this email directly or view it on GitHubhttps://github.com//issues/37#issuecomment-24746096
.

@BjarniRunar
Copy link
Member

@vibragiel : Thank you for the generous offer. I only hesitate to take you up on it because we are still doing extensive refactoring and stuff is getting reorganized a lot. We may circle back and accept your offer in a couple of weeks if/when things settle down a bit.

@markberger : Agreed!

@vibragiel
Copy link

@BjarniRunar Okay, we can do that. I'm used to convert code into PEP8, so I don't think it will take too long to get all of Mailpile worked out. Just give me a whistle when you feel ready :-)

@jeacaveo
Copy link

jeacaveo commented Nov 1, 2013

I would like to help with this task. Please let me know if you need any help on this matter.

@BjarniRunar
Copy link
Member

Thanks @jventuravs !

However, as discussed in #244, we have come to the conclusion that it is better for the core team to do the majority of this work, all the changes need to be reviewed by a core member for security reasons and for routine and semi-automated changes that most pep8 fixes are, it's actually a lot easier for us to just do the work ourselves than review and merge pull requests.

We have actually already made good progress, about half the files now comply with the tests from the Debian 7.2 version of the pep8 tool.

@jeacaveo
Copy link

jeacaveo commented Nov 1, 2013

Great, thanks for the heads up. Will be looking for some other way to contribute. Great project by the way, can't wait to use it!

@kmike
Copy link

kmike commented Nov 11, 2013

FYI: a collegue of mine showed a trick to remove whitespace changes from github diff: add w=1 to URL. Compare https://github.com/pagekite/Mailpile/pull/62/files?w=1 and https://github.com/pagekite/Mailpile/pull/62/files

@BjarniRunar
Copy link
Member

As of commit e1dd83e, the pep8 tool as shipped with Ubuntu 13.10 has virtually no complaints. I am closing this issue, we'll periodically rerun the tool and fix things that drift out of line.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants