Re: Conforming to pep8

On Thu, May 08, 2014 at 11:36:29PM -0500, Felipe Contreras wrote:

William Giokas wrote:

quoted

E401: Multi-line imports seems like something that would just be
changing one line

Yes, and make the code very annoying.

It's 1 extra line in git-remote-hg, and 4 lines in git-remote-bzr. I'll
even send in patches for the current version you have in your tree if
you'd like. It would make testing removal of an import easier, and the
adding of imports would be consistent. They can still be grouped, but
having them on a single line kind of limits the ease of manipulation for
testing and possible packager patching if needed.

quoted

E302: Blank lines don't seem to be that hard to do either. That can even
be automated quite reliably. It shouldn't detract from the readability,
juts makes the file a bit longer.

The problem is not that it's hard to do, the problem is that it makes
the code uglier.

I would disagree, but this is one of the less important things.

quoted

E20{1,2,3}: Extra whitespace is something that just makes things more
consistent and readable.

I don't see how this:

  {'100755': 'x', '120000': 'l'}

Is more readable than this:

  { '100755': 'x', '120000': 'l' }

No strong opinion on this one though.

It's not so much that it's wrong or less readable, but there is
inconsistency on this one and I'd err pep8. Again, will send a patch to
your tree for you to review, though it looks like you mostly fixed this
in [1].

quoted

E12{6,8}: continuation line indenting is another thing that helps
consistency.

I don't see how.

quoted

quoted

  max-line-length = 160

The standard states that this should, at most, be increased to a value
between 80 and 100.

And why's that?

This has been discussed many times in the LKML, and the end result is
that we don't live in the 60's, our terminals are not constrained to 60
characters. Going beyond 100 is fine.

Fair enough. At the same time, it'd only change 14 lines in the current
git.git tree and would probably increase the readability of some of the
sections. I noticed that some of the changes in the referenced patch
actually fixed this on a few lines as well.

[1]: https://github.com/felipec/git/commit/12374c0e09a84945a202bb5ba7981a223d233d0b

Thanks,
-- 
William Giokas | KaiSforza | http://kaictl.net/
GnuPG Key: 0x73CD09CF
Fingerprint: F73F 50EF BBE2 9846 8306  E6B8 6902 06D8 73CD 09CF