The implementation looks *really* nice, specially the idea of
moving getMirrors() to the base channel.
I'm adding some comments below, but it would be fantastic to
get this in place on time for 1.2:
[1]
The existence of MirrorsChannel doesn't seem to be necessary
anymore. Can we remove it?
[2]
This change will blow up any existing caches right after Smart
is upgraded. Talk to me online for hints about how to prevent
it.
[3]
The number of fetch steps will be wrong when there's no mirror file.
[4]
There are no tests for the changes in control.py
[5]
It would be great if the test for the channel was in a Python file
(e.g. metadata.py), specifically for testing the metadata channel,
and if this branch didn't change metadata.txt. The doctest file
was mostly a quick workaroud to get some tests in place, but it's
a poor way to verify incremental changes like this. For instance:
by introducing the test for mirrorlist, you're removing the test
that checks that it works fine *without* this option.
To get some idea for how to build this test file, check
tests/debloader.py, or just talk to me online. I'll be happy
to discuss this with you, or to help you.
The implementation looks *really* nice, specially the idea of
moving getMirrors() to the base channel.
I'm adding some comments below, but it would be fantastic to
get this in place on time for 1.2:
[1]
The existence of MirrorsChannel doesn't seem to be necessary
anymore. Can we remove it?
[2]
This change will blow up any existing caches right after Smart
is upgraded. Talk to me online for hints about how to prevent
it.
[3]
The number of fetch steps will be wrong when there's no mirror file.
[4]
There are no tests for the changes in control.py
[5]
It would be great if the test for the channel was in a Python file
(e.g. metadata.py), specifically for testing the metadata channel,
and if this branch didn't change metadata.txt. The doctest file
was mostly a quick workaroud to get some tests in place, but it's
a poor way to verify incremental changes like this. For instance:
by introducing the test for mirrorlist, you're removing the test
that checks that it works fine *without* this option.
To get some idea for how to build this test file, check
tests/debloader.py, or just talk to me online. I'll be happy
to discuss this with you, or to help you.