Autopep8 style whitespace cleanups & pre-commit hook

[craigez]

I've got a couple bug fixes I'm keen to commit, but I was wondering if you would accept a PR for a pep8 update as the trailing whitespace, etc... makes it a pain when your environment is set up to eliminate such things. If not I'll just start submitting the small fixes with your current formatting.

I also added a really basic pre-commit hook to check pep8 style, wasn't sure how you would want it installed. Only place I could see for now was doing it in setup.py.

[Byron]

Thanks for the contribution, a massive one indeed.
Something I don't like about it is the changes to the line length. Internally, I work with 120 characters, which is enough to fit two buffers side by side on a full-HD screen at a moderate font size. For some reason I am unwilling to keep code readable for those still working on a VT 100 ;).

Something I am particularly concerned about is the modifications to setup.py. Won't they break any windows installation ? Git-python traditionally ran on this unholy platform, and I wouldn't want to break installation for those users.
Also, pre-commit.sh uses pep8 without checking if it is actually installed (or in the PATH),which will fail ungracefully whenever it isn't. For now, I would want to consider integrating this tool optional, which shouldn't fail if it's not available.

[craigez]

I can re-do it without the line length alterations, no worries. I just ran autopep8. :)

As for windows, the shell script will break that too. :) Maybe we need something more complex then. Let me think about that.

I would suggest against considering pep8 optional. Either standardize on pep8, fix all the issues and add a pre-commit hook to keep it that way. Or don't worry about it at all. I'm not sure there's much value in fixing a coding standard but not enforcing it.

Seems this'll need a little more work, so I'll submit the other fixes I have separately while this is discussed & modified. Thanks for the feedback.

[Byron]

If I were you, I wouldn't spend any time to enforce automation of pep8, as you cannot possibly make it run or enforce it anywhere.
The way I think this can be done is to setup contribution guidelines which specify how code should be formatted, thus encouraging it using social means. One could, for instance, suggest tools or plugins that help doing this automatically, like this one.

In the end, such contribution guidelines should include a rule stating that prior to making a new release, pep8 needs to be run on all source files. That way, contributions can be made more easily, and the overall workflow will be faster, as the maintainer still can use github auto-merge.

As you can see, many things you, and I, run into now should be should be specified somewhere in the project. In general, I would like to align with this spec, sometime in the future.

[hashar]

You can use tox to provide an easy way for developers to run pep8 with a specific version of pep8 and custom rules. That can then be added in Travis-ci to automatically test pull requests.

I highly recommend using flake8 which is a wrapper around both pep8 and pyflakes (the later catching other issues) and also provides its own checks.

As for 80 versus 120 characters lines. I do all my coding in 80 columns terminal :-D

[craigez]

I changed this branch to just be the whitespace changes with a max line length of 120.

[Byron]

Thank you ! In order to assure your valuable hints will not be lost in a closed pull request, I am cross-referencing it in this issue.

That way, the required changes can be made at some point.

Argh, it seems a previous merge is now causing conflicts, preventing an automatic merge. I shall have a look some time later today.

[craigez]

I can rebase it later if I get a chance.

[Byron]

Great, thanks !

[craigez]

Done, rebased.

[Byron]

Thanks very much for your help. I am quite busy with another project right now.

[hashar]

Would you mind applying autopep8 (with the same settings) on the 0.3 branch as well?

I have proposed a patch on branch 0.3 to bring tox + flake8 to easily check the repo is compliant (PR #179 ).

[craigez]

No worries, I submitted PR #182 which is autopep8 with max length of 120 run against 0.3 branch.