Comment 10 for bug 247351

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

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.