1 |
On Wed, 10 Jun 2015 22:43:38 -0700 |
2 |
Zac Medico <zmedico@g.o> wrote: |
3 |
|
4 |
> On 06/10/2015 10:39 PM, Mike Frysinger wrote: |
5 |
> > On 10 Jun 2015 11:54, Zac Medico wrote: |
6 |
> >> On 05/30/2015 01:59 PM, Mike Frysinger wrote: |
7 |
> >> LGTM, except this one line is indented with spaces instead of tabs |
8 |
> >> in vartree.py: |
9 |
> >> |
10 |
> >>> def tar_contents(contents, root, tar, protect=None, |
11 |
> >>> onProgress=None, |
12 |
> >>> - xattr=False): |
13 |
> >>> + xattrs=False): |
14 |
> > |
15 |
> > i don't know if we have a standard here. sometimes it's a single |
16 |
> > tab, sometimes it's spaces to line up. i prefer the latter as |
17 |
> > that's generally what PEP8 does. -mike |
18 |
> > |
19 |
> |
20 |
> Ah, ok. The test pass, so guess it's fine that way. |
21 |
|
22 |
|
23 |
But I still don't like it being mixed. It may not be a problem due to |
24 |
it being contained inside the def statement. But I've had some strange |
25 |
results when code is run with mixed spaces/tabs indents. |
26 |
|
27 |
Portage code is not nearly pep8 and we have not set pep8 as a standard |
28 |
for us to adhere to. Although, I personally try to keep my stuff close |
29 |
to pep8. Aside from the fact I prefer a few things different than that |
30 |
standard. |
31 |
|
32 |
In this case, it is minor, but 1 tab indent (which is normal) also does |
33 |
not show the continuation of the def readily. I have at times just |
34 |
given it an additional indent tab to differentiate it from the following |
35 |
code. I personally don't adhere to the line up all params with the |
36 |
|
37 |
foo(param1, |
38 |
param2) |
39 |
|
40 |
It's fine when it is close to the left side, but when the |
41 |
start ( is near the right side, it can be down right |
42 |
impossible to stay within the 80 col. limit for long |
43 |
parameters. Besides the often times wasted space of having |
44 |
all params on separate lines. IMHO it can take away from |
45 |
readability at times while increasing readability for complex |
46 |
function calls. |
47 |
|
48 |
I've come across code in portage with both a mix of tabs and |
49 |
spaces on the same line(s)... In such cases, I have my editor convert |
50 |
all leading spaces to tabs. I don't look for odd cases like the above. |
51 |
But I will usually take note of them in the commit diff while making the |
52 |
commit. |
53 |
|
54 |
Personally, I'd prefer you stay with tabs, even if it does not |
55 |
line up exactly with the ( ignoring personal preferences for |
56 |
tab settings ( 1 tab = 4 spaces,...) |
57 |
|
58 |
Yes, I very much like the code being centralized in one place. :) |
59 |
|
60 |
We can look at any more convention changes after the next |
61 |
lead election. |
62 |
-- |
63 |
Brian Dolbec <dolsen> |