1 |
On Sat, 12 Dec 2015 13:01:04 +0100 |
2 |
Alexander Berntsen <bernalex@g.o> wrote: |
3 |
|
4 |
> Friends, |
5 |
> |
6 |
> As I'm sure you've noticed, Arfrever pushed a bunch of unreviewed |
7 |
> stuff. Michał reverted some of them as they were breaking changes in |
8 |
> addition to being unreviewed (oh, and of course they don't follow the |
9 |
> commit message format -- why would they). There's a bunch left. |
10 |
> |
11 |
> Do we want to cherry-pick Zac's patches (i.e. revert Arfrever's |
12 |
> patches), and have him put them up for review as usual? It's a bit of |
13 |
> a mess because of Zac's patches. If we go down this route it would |
14 |
> likely be easiest for Zac to revert them, since he'll be the more |
15 |
> confident regarding what to keep in the ensuing merge conflicts. |
16 |
> |
17 |
> Arfrever asked that someone review the patches and keep them if they |
18 |
> are OK, and revert only the ones that are problematic. I think this |
19 |
> would be OK for now, but obviously not something we want to encourage |
20 |
> or allow in the future. |
21 |
|
22 |
Here's my quick review of the remaining commits (newest first): |
23 |
|
24 |
|
25 |
31923f4 portage.package.ebuild.config.config.__init__(): Skip some warnings for Portage Python scripts called in ebuild environment. |
26 |
7853950 Delete support for PORTDIR and PORTDIR_OVERLAY from make.conf and environment. |
27 |
e7d95cb portage.repository.config.RepoConfig: Support location with trailing whitespace by using quoting. |
28 |
|
29 |
Those three already reverted. |
30 |
|
31 |
|
32 |
8036223 _emerge.depgraph.depgraph._select_files(): Update message. |
33 |
|
34 |
Can probably be left, it's updating messages for repos.conf which is still valid. |
35 |
|
36 |
|
37 |
9716f8a emirrordist: Delete support for deprecated --portdir and --portdir-overlay options. |
38 |
2d40eb4 egencache: Delete support for deprecated --portdir and --portdir-overlay options. |
39 |
|
40 |
Not sure. Maybe something relies on this. |
41 |
|
42 |
|
43 |
fb4d1f4 ebuild: Rename some variables. |
44 |
|
45 |
I'd keep it, doesn't hurt anything and the new names are more readable than 'myrepo' ;-). |
46 |
|
47 |
|
48 |
9e104c4 ebuild: Set PORTAGE_REPOSITORIES instead of deprecated PORTDIR_OVERLAY. |
49 |
|
50 |
Kill it as relying on weirdo PORTAGE_REPOSITORIES invention. |
51 |
|
52 |
|
53 |
3917544 bin/socks5-server.py: Fix DeprecationWarning with Python >=3.4.4. |
54 |
|
55 |
This one looks good, and even the commit message is sane. |
56 |
|
57 |
|
58 |
2cde1f6 portage.repository.config: Clean reading of repository names and drop support for repositories with missing or invalid names. |
59 |
|
60 |
I'd revert it, it's a breaking change. |
61 |
|
62 |
|
63 |
1064765 portage.repository.config.RepoConfig: Delete user_location attribute and consistently use location attribute everywhere. |
64 |
|
65 |
I think we can keep this if it doesn't break anything. I didn't hear |
66 |
about this attribute before, so I suggest Zac reviews this. |
67 |
|
68 |
|
69 |
48c2af4 Respect PYTHONDONTWRITEBYTECODE in test suite. |
70 |
|
71 |
I don't mind either way. But can't we make this simpler? |
72 |
|
73 |
|
74 |
528dc7c portage.package.ebuild.fetch.fetch(): Clean setting of variables. |
75 |
|
76 |
Purely stylistic change. |
77 |
|
78 |
|
79 |
10cccf7 Support environmental variables overriding parts of configuration of repositories. |
80 |
|
81 |
Kill this ugly thing with a lot of fire. This is so wrong you can't get |
82 |
much worse. |
83 |
|
84 |
|
85 |
39d81c5 portage.package.ebuild.config.config.__getitem__(): Partially drop backward compatibility for nonexistent keys. |
86 |
|
87 |
Maybe keep it but needs someone smarter than me to review. |
88 |
|
89 |
|
90 |
4f25fa6 ebuild: Move imports to the top. |
91 |
5ea789a portage.package.ebuild._config.LicenseManager.LicenseManager._getPkgAcceptLicense(): Fix typo in call to hasattr(). |
92 |
|
93 |
Makes sense. |
94 |
|
95 |
|
96 |
3ffdbbe ebuild: Do not catch unexpected KeyErrors from aux_get(). |
97 |
|
98 |
Maybe. Need others to confirm. |
99 |
|
100 |
|
101 |
b8da04b portageq envvar: Return 1 for any nonexistent variable. |
102 |
|
103 |
Makes sense. |
104 |
|
105 |
|
106 |
53a0b63 _emerge.actions.getgccversion(): Work with chost=None and with no explicit chost. |
107 |
|
108 |
Probably makes sense, needs extra review. |
109 |
|
110 |
|
111 |
628aa10 portage.sync.modules.cvs.CheckCVSConfig.check_cvs_repo(): Fix "KeyError: 'sync-cvs-repo'". |
112 |
|
113 |
Makes sense. |
114 |
|
115 |
|
116 |
f05056c portage.repository.config.RepoConfigLoader._parse(): Delete unused portdir parametre. |
117 |
041fc70 portage.repository.config.RepoConfigLoader._add_repositories(): Delete unused ignored_location_map parametre. |
118 |
c1a2714 portage.repository.config.RepoConfigLoader._parse(): Delete unused ignored_map and ignored_location_map parametres. |
119 |
|
120 |
Those all make sense. |
121 |
|
122 |
-- |
123 |
Best regards, |
124 |
Michał Górny |
125 |
<http://dev.gentoo.org/~mgorny/> |