Re: Conforming to pep8

From: William Giokas <1007380@gmail.com>
Date: May 9, 2014, 05:16 UTC
Message-ID: <20140509051623.GB9051@wst420>
In-Reply-To: <536c5b4d9e2c9_377dfcb2f02b@nysa.notmuch>

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

Show 5 quoted lines

> William Giokas wrote:
> > 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.

Show 6 quoted lines

> > 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.

Show 12 quoted lines

> > 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].

Show 6 quoted lines

> 
> > E12{6,8}: continuation line indenting is another thing that helps
> > consistency.
> 
> I don't see how.
>

Show 10 quoted lines

> > >   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