1 |
On Friday 17 January 2014 18:03:57 Tom Wijsman wrote: |
2 |
> --- |
3 |
|
4 |
please shorten your commit summary and move more content to the body |
5 |
|
6 |
> +getNonSystemArchiveDepends.archivers = { |
7 |
|
8 |
it is super weird to attach to the object like this. some might even say |
9 |
wrong. move it into the class definition. |
10 |
|
11 |
def getNonSystemArchiveDepends(fetchlist, eapi): |
12 |
... |
13 |
|
14 |
ARCHIVERS = { |
15 |
... |
16 |
} |
17 |
|
18 |
> + re.compile('.*\.7[zZ]$'):"app-arch/p7zip", |
19 |
|
20 |
regexes should always use raw strings. there should also be a space after the |
21 |
colon in dicts. so you want: |
22 |
|
23 |
re.compile(r'.*\.7[zZ]$'): 'app-arch/p7zip', |
24 |
|
25 |
> + re.compile('.*\.lzma$'):"app-arch/lzma-utils", |
26 |
|
27 |
xz-utils, not lzma-utils |
28 |
|
29 |
> + re.compile('.*\.(bz2?|tbz2)$'):"app-arch/bzip2", |
30 |
> + re.compile('.*\.(tar(\.(bz2?|gz|Z))?|tbz2|t[bg]z)?$'):"app-arch/tar", |
31 |
> + re.compile('.*\.(gz|tar\.Z|t[bg]z|[zZ])$'):"app-arch/gzip", |
32 |
> + re.compile('.*\.tar.xz$'):"app-arch/tar", |
33 |
|
34 |
requiring people list gzip, tar, and bzip2 is a significant policy change |
35 |
(which i'm inclined to say is wrong). it needs discussion on the dev mailing |
36 |
list first. |
37 |
|
38 |
> +def checkArchiveDepends(atoms, catpkg, relative_path, \ |
39 |
> + system_set_atoms, needed_unpack_depends, stats, fails): |
40 |
|
41 |
you don't need the \ there because you have paren already to gather things. |
42 |
|
43 |
> + for entry in needed_unpack_depends[catpkg]: |
44 |
> + if entry not in [atom.cp for atom in atoms if atom != "||"]: |
45 |
> + stats['unpack.' + mytype + '.missing'] += 1 |
46 |
> + fails['unpack.' + mytype + '.missing'].append( \ |
47 |
> + relative_path + ": %s is missing in %s" % \ |
48 |
> + (entry, mytype)) |
49 |
|
50 |
i know existing style doesn't follow it, but imo we should avoid string |
51 |
concatenation. it makes the code harder to read imo. |
52 |
|
53 |
key = 'unpack.%s.missing' % mytype |
54 |
stats[key] += 1 |
55 |
fails[key].append(...) |
56 |
|
57 |
> +def eapi_has_xz_utils(eapi): |
58 |
> + return eapi not in ("0", "1", "2") |
59 |
|
60 |
i would use "eapi_supports_xz_archives" unless there's a pre-existing style |
61 |
-mike |