On Sat, 12 Dec 2015 13:01:04 +0100
Alexander Berntsen <berna...@gentoo.org> wrote:

> Friends,
> 
> As I'm sure you've noticed, Arfrever pushed a bunch of unreviewed
> stuff. Michał reverted some of them as they were breaking changes in
> addition to being unreviewed (oh, and of course they don't follow the
> commit message format -- why would they). There's a bunch left.
> 
> Do we want to cherry-pick Zac's patches (i.e. revert Arfrever's
> patches), and have him put them up for review as usual? It's a bit of
> a mess because of Zac's patches. If we go down this route it would
> likely be easiest for Zac to revert them, since he'll be the more
> confident regarding what to keep in the ensuing merge conflicts.
> 
> Arfrever asked that someone review the patches and keep them if they
> are OK, and revert only the ones that are problematic. I think this
> would be OK for now, but obviously not something we want to encourage
> or allow in the future.

Here's my quick review of the remaining commits (newest first):


31923f4 portage.package.ebuild.config.config.__init__(): Skip some warnings for 
Portage Python scripts called in ebuild environment.
7853950 Delete support for PORTDIR and PORTDIR_OVERLAY from make.conf and 
environment.
e7d95cb portage.repository.config.RepoConfig: Support location with trailing 
whitespace by using quoting.

Those three already reverted.


8036223 _emerge.depgraph.depgraph._select_files(): Update message.

Can probably be left, it's updating messages for repos.conf which is still 
valid.


9716f8a emirrordist: Delete support for deprecated --portdir and 
--portdir-overlay options.
2d40eb4 egencache: Delete support for deprecated --portdir and 
--portdir-overlay options.

Not sure. Maybe something relies on this.


fb4d1f4 ebuild: Rename some variables.

I'd keep it, doesn't hurt anything and the new names are more readable than 
'myrepo' ;-).


9e104c4 ebuild: Set PORTAGE_REPOSITORIES instead of deprecated PORTDIR_OVERLAY.

Kill it as relying on weirdo PORTAGE_REPOSITORIES invention.


3917544 bin/socks5-server.py: Fix DeprecationWarning with Python >=3.4.4.

This one looks good, and even the commit message is sane.


2cde1f6 portage.repository.config: Clean reading of repository names and drop 
support for repositories with missing or invalid names.

I'd revert it, it's a breaking change.


1064765 portage.repository.config.RepoConfig: Delete user_location attribute 
and consistently use location attribute everywhere.

I think we can keep this if it doesn't break anything. I didn't hear
about this attribute before, so I suggest Zac reviews this.


48c2af4 Respect PYTHONDONTWRITEBYTECODE in test suite.

I don't mind either way. But can't we make this simpler?


528dc7c portage.package.ebuild.fetch.fetch(): Clean setting of variables.

Purely stylistic change.


10cccf7 Support environmental variables overriding parts of configuration of 
repositories.

Kill this ugly thing with a lot of fire. This is so wrong you can't get
much worse.


39d81c5 portage.package.ebuild.config.config.__getitem__(): Partially drop 
backward compatibility for nonexistent keys.

Maybe keep it but needs someone smarter than me to review.


4f25fa6 ebuild: Move imports to the top.
5ea789a 
portage.package.ebuild._config.LicenseManager.LicenseManager._getPkgAcceptLicense():
 Fix typo in call to hasattr().

Makes sense.


3ffdbbe ebuild: Do not catch unexpected KeyErrors from aux_get().

Maybe. Need others to confirm.


b8da04b portageq envvar: Return 1 for any nonexistent variable.

Makes sense.


53a0b63 _emerge.actions.getgccversion(): Work with chost=None and with no 
explicit chost.

Probably makes sense, needs extra review.


628aa10 portage.sync.modules.cvs.CheckCVSConfig.check_cvs_repo(): Fix 
"KeyError: 'sync-cvs-repo'".

Makes sense.


f05056c portage.repository.config.RepoConfigLoader._parse(): Delete unused 
portdir parametre.
041fc70 portage.repository.config.RepoConfigLoader._add_repositories(): Delete 
unused ignored_location_map parametre.
c1a2714 portage.repository.config.RepoConfigLoader._parse(): Delete unused 
ignored_map and ignored_location_map parametres.

Those all make sense.

-- 
Best regards,
Michał Górny
<http://dev.gentoo.org/~mgorny/>

Attachment: pgpvMFEuGVeiX.pgp
Description: OpenPGP digital signature

Reply via email to