| 1 |
On 03/29/2016 01:35 AM, Alexander Berntsen wrote: |
| 2 |
> On 20/03/16 15:33, Adam Mills wrote: |
| 3 |
>> + for set_name in root_config.sets: |
| 4 |
>> + if set_name == "system": |
| 5 |
>> + system = root_config.sets[set_name].findAtomForPackage( |
| 6 |
>> + pkg, modified_use=self.conf.pkg_use_enabled(pkg)) |
| 7 |
>> + elif set_name == "selected": |
| 8 |
>> + world = root_config.sets[set_name].findAtomForPackage( |
| 9 |
>> + pkg, modified_use=self.conf.pkg_use_enabled(pkg)) |
| 10 |
>> + elif user_set is None and root_config.sets[set_name].isUserSet(): |
| 11 |
>> + user_set = root_config.sets[set_name].findAtomForPackage( |
| 12 |
>> + pkg, modified_use=self.conf.pkg_use_enabled(pkg)) |
| 13 |
> This is really silly. At the very least try to not repeat yourself so |
| 14 |
> much. |
| 15 |
|
| 16 |
In order to avoid the loop, all of the user sets could be combined into |
| 17 |
a single InternalPackageSet instance, for indexing purposes. |
| 18 |
|
| 19 |
As is, the logic is wrong because the user_set should be an accumulator, |
| 20 |
assigned with an operator like |= since you don't what this variable to |
| 21 |
toggle back and forth in the loop. |
| 22 |
|
| 23 |
> The patch otherwise looks OK, and the idea is OK by me. Unless someone |
| 24 |
> has any big objections, I can probably merge a cleaned up patch. |
| 25 |
|
| 26 |
As discussed in the meeting today, the above loop needs to be fixed |
| 27 |
before we can merge this. |
| 28 |
-- |
| 29 |
Thanks, |
| 30 |
Zac |
Archives